Skip to content

Commit

Permalink
main,monitor: fix total steps in progress reporting
Browse files Browse the repository at this point in the history
The existing code to record progress was a bit too naive. Instead
of just counting the number os pipelines in a manifest to get the
total steps we need to look at the resolved pipelines.

with this fix `bib` will report the correct number of steps left
when doing e.g. a qcow2 image build. Right now the number of
steps is incorrect because the osbuild manifest contains pipelines
for qcow2,vdmk,raw,ami and all are currently considered steps
that need to be completed. With this commit this is fixed.
  • Loading branch information
mvo5 committed Jul 30, 2024
1 parent 46db834 commit fbcb0f9
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 12 deletions.
5 changes: 3 additions & 2 deletions osbuild/main_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,6 @@ def osbuild_cli() -> int:
monitor_name = args.monitor
if not monitor_name:
monitor_name = "NullMonitor" if args.json else "LogMonitor"
monitor = osbuild.monitor.make(monitor_name, args.monitor_fd, manifest)
monitor.log(f"starting {args.manifest_path}", origin="osbuild.main_cli")

try:
with ObjectStore(args.store) as object_store:
Expand All @@ -175,6 +173,9 @@ def osbuild_cli() -> int:
debug_break = args.debug_break

pipelines = manifest.depsolve(object_store, exports)
total_steps = len(manifest.sources) + len(pipelines)
monitor = osbuild.monitor.make(monitor_name, args.monitor_fd, total_steps)
monitor.log(f"starting {args.manifest_path}", origin="osbuild.main_cli")

manifest.download(object_store, monitor, args.libdir)

Expand Down
17 changes: 8 additions & 9 deletions osbuild/monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -211,10 +211,9 @@ def write(self, text: str):
class BaseMonitor(abc.ABC):
"""Base class for all pipeline monitors"""

def __init__(self, fd: int, manifest: Optional[osbuild.Manifest] = None) -> None:
def __init__(self, fd: int, _: int = 0) -> None:
"""Logging will be done to file descriptor `fd`"""
self.out = TextWriter(fd)
self.manifest = manifest

def begin(self, pipeline: osbuild.Pipeline):
"""Called once at the beginning of a pipeline"""
Expand Down Expand Up @@ -251,8 +250,8 @@ class LogMonitor(BaseMonitor):
sequences will be used to highlight sections of the log.
"""

def __init__(self, fd: int, manifest: Optional[osbuild.Manifest] = None):
super().__init__(fd, manifest)
def __init__(self, fd: int, total_steps: int = 0):
super().__init__(fd, total_steps)
self.timer_start = 0

def result(self, result):
Expand Down Expand Up @@ -308,10 +307,10 @@ def log(self, message, origin: Optional[str] = None):
class JSONSeqMonitor(BaseMonitor):
"""Monitor that prints the log output of modules wrapped in json-seq objects with context and progress metadata"""

def __init__(self, fd: int, manifest: osbuild.Manifest):
super().__init__(fd, manifest)
def __init__(self, fd: int, total_steps: int):
super().__init__(fd, total_steps)
self._ctx_ids: Set[str] = set()
self._progress = Progress("pipelines/sources", len(manifest.pipelines) + len(manifest.sources))
self._progress = Progress("pipelines/sources", total_steps)
self._context = Context(origin="org.osbuild")

def begin(self, pipeline: osbuild.Pipeline):
Expand Down Expand Up @@ -353,11 +352,11 @@ def _jsonseq(self, entry):
self.out.write("\n")


def make(name, fd, manifest):
def make(name: str, fd: int, total_steps: int) -> BaseMonitor:
module = sys.modules[__name__]
monitor = getattr(module, name, None)
if not monitor:
raise ValueError(f"Unknown monitor: {name}")
if not issubclass(monitor, BaseMonitor):
raise ValueError(f"Invalid monitor: {name}")
return monitor(fd, manifest)
return monitor(fd, total_steps)
2 changes: 1 addition & 1 deletion test/mod/test_monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ def test_json_progress_monitor():
manifest.add_pipeline(pl2, "", "")

with tempfile.TemporaryFile() as tf:
mon = JSONSeqMonitor(tf.fileno(), manifest)
mon = JSONSeqMonitor(tf.fileno(), len(manifest.sources)+len(manifest.pipelines))
mon.log("test-message-1")
mon.log("test-message-2", origin="test.origin.override")
mon.begin(manifest.pipelines["test-pipeline-first"])
Expand Down

0 comments on commit fbcb0f9

Please sign in to comment.