From fbcb0f9a5158e72018a4b24b7ce287e9c7bcc844 Mon Sep 17 00:00:00 2001 From: Michael Vogt Date: Tue, 30 Jul 2024 11:46:29 +0200 Subject: [PATCH] main,monitor: fix total steps in progress reporting 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. --- osbuild/main_cli.py | 5 +++-- osbuild/monitor.py | 17 ++++++++--------- test/mod/test_monitor.py | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/osbuild/main_cli.py b/osbuild/main_cli.py index 28d5f740b..3afd97c4a 100644 --- a/osbuild/main_cli.py +++ b/osbuild/main_cli.py @@ -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: @@ -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) diff --git a/osbuild/monitor.py b/osbuild/monitor.py index fd1201c78..eebc4cc19 100644 --- a/osbuild/monitor.py +++ b/osbuild/monitor.py @@ -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""" @@ -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): @@ -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): @@ -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) diff --git a/test/mod/test_monitor.py b/test/mod/test_monitor.py index 7626d9fb7..77e20ec3e 100644 --- a/test/mod/test_monitor.py +++ b/test/mod/test_monitor.py @@ -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"])