monitor: tweak/simplify Progress

Tweak the Progress class to be simpler. Given that progress does
not need to support arbitrary depth but only has a single level
the class now just exposes "sub_progress" to the caller.

When the main progress is advanced the sub_progress is now fully
deleted instead of just reset. The rational is that when the main
progress is done and advances a step it is very likely that a
new sub_progress is required and it's most likely an error if
the same sub_progress will get re-used.

This means that `reset()` can be removed as it's not used anymore
(and YAGNI). We can add it back when we have a use-case.

It also change the code so that "total" starts with 0 instead
of `None` (principle of least surprise). This means that now
`progress.incr()` is called in the JSONSeqMonitor() for
`finish()` and `result()` to indicate that the pipeline/stage
is finished.

osbuild/monitor.py

@@ -113,30 +113,29 @@ class Context:
 
 
 class Progress:
+    """Progress represents generic progress information.
+
+    A progress can contain a sub_progress to track more
+    nested progresses. Any increment of a parent progress
+    will the reset the sub_progress to None and a new
+    sub_progress needs to be provided.
+
+    Keyword arguments:
+    name  -- user visible name for the progress
+    total -- total steps required to finish the progress
+    """
+
     def __init__(self, name: str, total: int):
         self.name = name
         self.total = total
-        self.done = None
+        self.done = 0
-        self._sub_progress: Optional[Progress] = None
+        self.sub_progress: Optional[Progress] = None
 
-    def incr(self, depth=0):
+    def incr(self):
-        if depth > 0:
+        """Increment the "done" count"""
-            self._sub_progress.incr(depth - 1)
+        self.done += 1
-        else:
+        if self.sub_progress:
-            if self.done is None:
+            self.sub_progress = None
-                self.done = 0
-            else:
-                self.done += 1
-            if self._sub_progress:
-                self._sub_progress.reset()
-
-    def reset(self):
-        self.done = None
-        if self._sub_progress:
-            self._sub_progress.reset()
-
-    def sub_progress(self, prog: "Progress"):
-        self._sub_progress = prog
 
     def as_dict(self):
         d = {
@@ -144,8 +143,8 @@ class Progress:
             "total": self.total,
             "done": self.done,
         }
-        if self._sub_progress:
+        if self.sub_progress:
-            d["progress"] = self._sub_progress.as_dict()
+            d["progress"] = self.sub_progress.as_dict()
         return d
 
 
@@ -299,23 +298,28 @@ class JSONSeqMonitor(BaseMonitor):
         self._progress = Progress("pipelines", len(manifest.pipelines))
         self._context = Context(origin="org.osbuild")
 
-    def result(self, result):
+    # result is for modules
-        pass
+    def result(self, result: osbuild.pipeline.BuildResult):
+        # TODO: check pipeline id?
+        if self._progress.sub_progress:
+            self._progress.sub_progress.incr()
 
     def begin(self, pipeline: osbuild.Pipeline):
         self._context.set_pipeline(pipeline)
-        self._progress.sub_progress(Progress("stages", len(pipeline.stages)))
+        self._progress.sub_progress = Progress("stages", len(pipeline.stages))
+
+    # finish is for pipelines
+    def finish(self, result: dict):
         self._progress.incr()
 
     def stage(self, stage: osbuild.Stage):
         self._module(stage)
 
-    def assembler(self, assembler):
+    def assembler(self, assembler: osbuild.Stage):
         self._module(assembler)
 
-    def _module(self, module):
+    def _module(self, module: osbuild.Stage):
         self._context.set_stage(module)
-        self._progress.incr(depth=1)
 
     def log(self, message, origin: Optional[str] = None):
         oo = self._context.origin

test/mod/test_monitor.py

@@ -160,25 +160,27 @@ def test_context():
 
 def test_progress():
     prog = Progress("test", total=12)
-    subprog = Progress("test-sub1", total=3)
+    prog.sub_progress = Progress("test-sub1", total=3)
-    prog.sub_progress(subprog)
+    # we start empty
-    assert prog.done is None  # starts with None until the first incr()
-    prog.incr()
-
-    prog.incr(depth=1)
     progdict = prog.as_dict()
     assert progdict["done"] == 0
     assert progdict["progress"]["done"] == 0
 
-    prog.incr(depth=1)
+    # incr a sub_progress only affect sub_progress
+    prog.sub_progress.incr()
     progdict = prog.as_dict()
     assert progdict["done"] == 0
     assert progdict["progress"]["done"] == 1
 
+    prog.sub_progress.incr()
+    progdict = prog.as_dict()
+    assert progdict["done"] == 0
+    assert progdict["progress"]["done"] == 2
+
     prog.incr()
     progdict = prog.as_dict()
     assert progdict["done"] == 1
-    assert progdict["progress"]["done"] is None, "sub-progress did not reset"
+    assert progdict.get("progress") == None, "sub-progress did not reset"
 
 
 def test_json_progress_monitor():