chore: refactor legacy loop, fix error condition; note that status co…

…mpounding is ineffective

juju/application.py

@@ -584,10 +584,10 @@ async def get_status(self):
 
         full_status = await client_facade.FullStatus(patterns=None)
 
-        import pprint
-        with open("/tmp/full.status.jsonl", "a") as f:
-            print(file=f)
-            print(pprint.pformat(full_status.serialize()), file=f)
+        # import pprint
+        # with open("/tmp/full.status.jsonl", "a") as f:
+        #     print(file=f)
+        #     print(pprint.pformat(full_status.serialize()), file=f)
 
         _app = full_status.applications.get(self.name, None)
         if not _app:

juju/model.py

@@ -3061,7 +3061,6 @@ async def _check_idle(
         now = datetime.now()
         expected_idle_since = now - timedelta(seconds=idle_period)
         full_status = await self.get_status()
-        # __import__("pdb").set_trace()
 
         for app_name in apps:
             if not (app := full_status.applications.get(app_name)):
@@ -3207,7 +3206,6 @@ async def _legacy_check_idle(
         last_log_time: List[Optional[datetime]],  # = [None]
         start_time: datetime,
     ):
-        # __import__("pdb").set_trace()
         _timeout = timedelta(seconds=timeout) if timeout is not None else None
         _idle_period = timedelta(seconds=idle_period)
         log_interval = timedelta(seconds=30)
@@ -3250,16 +3248,16 @@ async def _legacy_check_idle(
             for unit in app.units:
                 if raise_on_error and unit.machine is not None and unit.machine.status == "error":
                     errors.setdefault("Machine", []).append(unit.machine.id)
-                    return False
+                    continue
                 if raise_on_error and unit.agent_status == "error":
                     errors.setdefault("Agent", []).append(unit.name)
-                    return False
+                    continue
                 if raise_on_error and unit.workload_status == "error":
                     errors.setdefault("Unit", []).append(unit.name)
-                    return False
+                    continue
                 if raise_on_blocked and unit.workload_status == "blocked":
                     blocks.setdefault("Unit", []).append(unit.name)
-                    return False
+                    continue
                 # TODO (cderici): we need two versions of wait_for_idle, one for waiting on
                 #  individual units, another one for waiting for an application.
                 #  The convoluted logic below is the result of trying to do both at the same

juju/status.py

@@ -10,14 +10,14 @@
 """ derive_status is used to determine the application status from a set of unit
 status values.
 
-:param statues: list of known unit workload statues
+:param statuses: list of known unit workload statuses
 
 """
 
 
-def derive_status(statues):
+def derive_status(statuses):
     current = 'unknown'
-    for status in statues:
+    for status in statuses:
         if status in severity_map and severity_map[status] > severity_map[current]:
             current = status
     return current

tests/integration/test_model.py

@@ -51,7 +51,6 @@ async def test_deploy_local_bundle_dir():
             for (k, v) in model.applications.items():
                 writer.write(k)
         assert app1 and app2
-        # import pdb;pdb.set_trace()
         await model.wait_for_idle(['juju-qa-test', 'nrpe'], wait_for_at_least_units=1)
 
 

tests/unit/test_wait_for_idle.py

@@ -17,7 +17,7 @@
     FullStatus,
     UnitStatus,
 )
-from juju.errors import JujuAppError
+from juju.errors import JujuAppError, JujuUnitError
 from juju.machine import Machine
 from juju.model import Model
 from juju.unit import Unit
@@ -60,6 +60,24 @@ async def test_raise_on_error(full_status_response: dict, kwargs: Dict[str, Any]
     assert "big problem" in str(idle)
 
 
+async def test_raise_on_error_compounding(full_status_response: dict, kwargs: Dict[str, Any]):
+    app = full_status_response["response"]["applications"]["hexanator"]
+    app["status"]["status"] = "unset"
+    app["units"]["hexanator/0"]["workload-status"]["status"] = "error"
+    app["units"]["hexanator/0"]["workload-status"]["info"] = "unit problem"
+    kwargs["apps"] = ["hexanator"]
+    kwargs["raise_on_error"] = True
+    idle, legacy = await model_fake(full_status_response).check_both(**kwargs)
+    assert isinstance(idle, JujuUnitError)
+    # FIXME looks like from the wait_for_idle point of view:
+    # app in unset means nothing because
+    # both app compound status is registered as error
+    # and unit direct status is registered as error
+    # and unit status raises first
+    assert isinstance(legacy, JujuUnitError)
+    assert "unit problem" in str(idle)
+
+
 async def test_raise_on_blocked(full_status_response: dict, kwargs: Dict[str, Any]):
     full_status_response["response"]["applications"]["hexanator"]["status"]["status"] = "blocked"
     full_status_response["response"]["applications"]["hexanator"]["status"]["info"] = "big problem"
@@ -178,10 +196,8 @@ def model_fake(resp: dict) -> ModelFake:
         assert fsapp
         assert fsapp.status  # DetailedStatus
         assert isinstance(fsapp.status.status, str)
-        app._status = fsapp.status.status
-
         assert isinstance(fsapp.status.info, str)
-        app._status_message = fsapp.status.info
+        app.set_status(fsapp.status.status, fsapp.status.info)
 
         for uname in fsapp.units:
             app._units.append(unit := UnitFake(uname, m))
@@ -243,25 +259,25 @@ async def rpc(self, msg, encoder=None):
 
 
 class ApplicationFake(Application):
-    _status: str = ""
-    _status_message: str = ""
+    _safe_data: dict
     _units: List[Unit]
 
-    @property
-    def status(self) -> str:
-        return self._status
-
-    @property
-    def status_message(self) -> str:
-        return self._status_message
+    def set_status(self, status="fixme", info="some info"):
+        self._safe_data["status"]["current"] = status
+        self._safe_data["status"]["message"] = info
 
     @property
     def units(self) -> List[Unit]:
         return self._units
 
+    @property
+    def safe_data(self) -> dict:
+        return self._safe_data
+
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self._units = []
+        self._safe_data = {"status": {"current": "fixme", "message": "fixme"}}
 
 
 class UnitFake(Unit):