From 03e7d5119103122acf21384e1cef40dbe7945145 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Tue, 26 Nov 2024 17:07:01 +0900 Subject: [PATCH 1/8] feat: new Model.wait_for_idle() --- .github/workflows/test.yaml | 9 +- juju/client/facade.py | 68 +++-- juju/{model.py => model/__init__.py} | 221 ++++++++++---- juju/model/_idle.py | 221 ++++++++++++++ juju/unit.py | 4 +- juju/version.py | 2 +- pyproject.toml | 8 +- setup.py | 1 + tests/unit/data/fullstatus.json | 322 ++++++++++++++++++++ tests/unit/data/subordinate-fullstatus.json | 254 +++++++++++++++ tests/unit/test_idle_check.py | 273 +++++++++++++++++ tests/unit/test_idle_check_subordinate.py | 57 ++++ tests/unit/test_idle_loop.py | 61 ++++ tests/unit/test_model.py | 17 +- tests/unit/test_unit.py | 2 +- tox.ini | 1 + 16 files changed, 1423 insertions(+), 98 deletions(-) rename juju/{model.py => model/__init__.py} (95%) create mode 100644 juju/model/_idle.py create mode 100644 tests/unit/data/fullstatus.json create mode 100644 tests/unit/data/subordinate-fullstatus.json create mode 100644 tests/unit/test_idle_check.py create mode 100644 tests/unit/test_idle_check_subordinate.py create mode 100644 tests/unit/test_idle_loop.py diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index dfe55d16..0073eebc 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -78,6 +78,9 @@ jobs: - "3.4/stable" - "3.5/stable" - "3.6/stable" + new_wait_for_idle: + - "True" + - "False" steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 @@ -116,7 +119,9 @@ jobs: # # set model defaults # juju model-defaults apt-http-proxy=$PROXY apt-https-proxy=$PROXY juju-http-proxy=$PROXY juju-https-proxy=$PROXY snap-http-proxy=$PROXY snap-https-proxy=$PROXY # juju model-defaults - - run: uvx -p ${{ matrix.python }} tox -e integration + - run: uvx -p ${{ matrix.python }} tox -s -e integration + env: + JUJU_NEW_WAIT_FOR_IDLE: ${{ matrix.new_wait_for_idle }} integration-quarantine: name: Quarantined Integration Tests @@ -144,4 +149,4 @@ jobs: with: provider: lxd juju-channel: ${{ matrix.juju }} - - run: uvx -p ${{ matrix.python }} tox -e integration-quarantine + - run: uvx -p ${{ matrix.python }} tox -s -e integration-quarantine diff --git a/juju/client/facade.py b/juju/client/facade.py index f0ee7513..7ab4fddd 100644 --- a/juju/client/facade.py +++ b/juju/client/facade.py @@ -14,7 +14,7 @@ from collections import defaultdict from glob import glob from pathlib import Path -from typing import Any, Mapping, Sequence +from typing import Any, Mapping, Sequence, TypeVar, overload import packaging.version import typing_inspect @@ -183,7 +183,7 @@ def ref_type(self, obj): return self.get_ref_type(obj["$ref"]) -CLASSES = {} +CLASSES: dict[str, type[Type]] = {} factories = codegen.Capture() @@ -479,37 +479,48 @@ def ReturnMapping(cls): # noqa: N802 def decorator(f): @functools.wraps(f) async def wrapper(*args, **kwargs): - nonlocal cls reply = await f(*args, **kwargs) - if cls is None: - return reply - if "error" in reply: - cls = CLASSES["Error"] - if typing_inspect.is_generic_type(cls) and issubclass( - typing_inspect.get_origin(cls), Sequence - ): - parameters = typing_inspect.get_parameters(cls) - result = [] - item_cls = parameters[0] - for item in reply: - result.append(item_cls.from_json(item)) - """ - if 'error' in item: - cls = CLASSES['Error'] - else: - cls = item_cls - result.append(cls.from_json(item)) - """ - else: - result = cls.from_json(reply["response"]) - - return result + return _convert_response(reply, cls=cls) return wrapper return decorator +@overload +def _convert_response(response: dict[str, Any], *, cls: type[SomeType]) -> SomeType: ... + + +@overload +def _convert_response(response: dict[str, Any], *, cls: None) -> dict[str, Any]: ... + + +def _convert_response(response: dict[str, Any], *, cls: type[Type] | None) -> Any: + if cls is None: + return response + if "error" in response: + cls = CLASSES["Error"] + if typing_inspect.is_generic_type(cls) and issubclass( + typing_inspect.get_origin(cls), Sequence + ): + parameters = typing_inspect.get_parameters(cls) + result = [] + item_cls = parameters[0] + for item in response: + result.append(item_cls.from_json(item)) + """ + if 'error' in item: + cls = CLASSES['Error'] + else: + cls = item_cls + result.append(cls.from_json(item)) + """ + else: + result = cls.from_json(response["response"]) + + return result + + def make_func(cls, name, description, params, result, _async=True): indent = " " args = Args(cls.schema, params) @@ -663,7 +674,7 @@ async def rpc(self, msg: dict[str, _RichJson]) -> _Json: return result @classmethod - def from_json(cls, data): + def from_json(cls, data: Type | str | dict[str, Any] | list[Any]) -> Type | None: def _parse_nested_list_entry(expr, result_dict): if isinstance(expr, str): if ">" in expr or ">=" in expr: @@ -742,6 +753,9 @@ def get(self, key, default=None): return getattr(self, attr, default) +SomeType = TypeVar("SomeType", bound=Type) + + class Schema(dict): def __init__(self, schema): self.name = schema["Name"] diff --git a/juju/model.py b/juju/model/__init__.py similarity index 95% rename from juju/model.py rename to juju/model/__init__.py index 31e93913..0063fbb6 100644 --- a/juju/model.py +++ b/juju/model/__init__.py @@ -1,10 +1,12 @@ # Copyright 2023 Canonical Ltd. # Licensed under the Apache V2, see LICENCE file for details. +"""Represent Juju Model, as in the workspace into which applications are deployed.""" + from __future__ import annotations import asyncio import base64 -import collections +import collections.abc import hashlib import json import logging @@ -13,6 +15,7 @@ import stat import sys import tempfile +import time import warnings import weakref import zipfile @@ -20,23 +23,26 @@ from datetime import datetime, timedelta from functools import partial from pathlib import Path -from typing import TYPE_CHECKING, Any, Literal, Mapping, overload +from typing import TYPE_CHECKING, Any, Iterable, Literal, Mapping, overload import websockets import yaml from typing_extensions import deprecated -from . import provisioner, tag, utils -from .annotationhelper import _get_annotations, _set_annotations -from .bundle import BundleHandler, get_charm_series, is_local_charm -from .charmhub import CharmHub -from .client import client, connection, connector -from .client.overrides import Caveat, Macaroon -from .constraints import parse as parse_constraints -from .constraints import parse_storage_constraints -from .controller import ConnectedController, Controller -from .delta import get_entity_class, get_entity_delta -from .errors import ( +from .. import provisioner, tag, utils +from ..annotationhelper import _get_annotations, _set_annotations +from ..bundle import BundleHandler, get_charm_series, is_local_charm +from ..charmhub import CharmHub +from ..client import client, connection, connector +from ..client._definitions import ApplicationStatus as ApplicationStatus +from ..client._definitions import MachineStatus as MachineStatus +from ..client._definitions import UnitStatus as UnitStatus +from ..client.overrides import Caveat, Macaroon +from ..constraints import parse as parse_constraints +from ..constraints import parse_storage_constraints +from ..controller import ConnectedController, Controller +from ..delta import get_entity_class, get_entity_delta +from ..errors import ( JujuAgentError, JujuAPIError, JujuAppError, @@ -49,27 +55,37 @@ JujuUnitError, PylibjujuError, ) -from .exceptions import DeadEntityException -from .names import is_valid_application -from .offerendpoints import ParseError as OfferParseError -from .offerendpoints import parse_local_endpoint, parse_offer_url -from .origin import Channel, Source -from .placement import parse as parse_placement -from .secrets import create_secret_data, read_secret_data -from .tag import application as application_tag -from .url import URL, Schema -from .version import DEFAULT_ARCHITECTURE +from ..exceptions import DeadEntityException +from ..names import is_valid_application +from ..offerendpoints import ParseError as OfferParseError +from ..offerendpoints import parse_local_endpoint, parse_offer_url +from ..origin import Channel, Source +from ..placement import parse as parse_placement +from ..secrets import create_secret_data, read_secret_data +from ..tag import application as application_tag +from ..url import URL, Schema +from ..version import DEFAULT_ARCHITECTURE +from . import _idle if TYPE_CHECKING: - from .application import Application - from .client._definitions import FullStatus - from .constraints import StorageConstraintDict - from .machine import Machine - from .relation import Relation - from .remoteapplication import ApplicationOffer, RemoteApplication - from .unit import Unit + from ..application import Application + from ..client._definitions import FullStatus + from ..constraints import StorageConstraintDict + from ..machine import Machine + from ..relation import Relation + from ..remoteapplication import ApplicationOffer, RemoteApplication + from ..unit import Unit + +log = logger = logging.getLogger(__name__) -log = logging.getLogger(__name__) + +def use_new_wait_for_idle() -> bool: + val = os.getenv("JUJU_NEW_WAIT_FOR_IDLE") + if not val: + return False + if val.isdigit(): + return bool(int(val)) + return val.title() != "False" class _Observer: @@ -632,9 +648,9 @@ class Model: def __init__( self, - max_frame_size=None, - bakery_client=None, - jujudata=None, + max_frame_size: int | None = None, + bakery_client: Any = None, + jujudata: Any = None, ): """Instantiate a new Model. @@ -2664,14 +2680,21 @@ async def get_action_status(self, uuid_or_prefix=None, name=None): results[tag.untag("action-", a.action.tag)] = a.status return results - async def get_status(self, filters=None, utc=False) -> FullStatus: + async def get_status(self, filters=None, utc: bool = False) -> FullStatus: """Return the status of the model. :param str filters: Optional list of applications, units, or machines to include, which can use wildcards ('*'). - :param bool utc: Display time as UTC in RFC3339 format + :param bool utc: Deprecated, display time as UTC in RFC3339 format """ + if utc: + warnings.warn( + "Model.get_status() utc= parameter is deprecated", + DeprecationWarning, + stacklevel=2, + ) + client_facade = client.ClientFacade.from_connection(self.connection()) return await client_facade.FullStatus(patterns=filters) @@ -2998,7 +3021,7 @@ async def _get_source_api(self, url): async def wait_for_idle( self, - apps: list[str] | None = None, + apps: Iterable[str] | None = None, raise_on_error: bool = True, raise_on_blocked: bool = False, wait_for_active: bool = False, @@ -3011,7 +3034,7 @@ async def wait_for_idle( ) -> None: """Wait for applications in the model to settle into an idle state. - :param List[str] apps: Optional list of specific app names to wait on. + :param Iterable[str]|None apps: Optional list of specific app names to wait on. If given, all apps must be present in the model and idle, while other apps in the model can still be busy. If not given, all apps currently in the model must be idle. @@ -3037,6 +3060,7 @@ async def wait_for_idle( units of all apps need to be `idle`. This delay is used to ensure that any pending hooks have a chance to start to avoid false positives. The default is 15 seconds. + Exact behaviour is undefined for very small values and 0. :param float check_freq: How frequently, in seconds, to check the model. The default is every half-second. @@ -3053,6 +3077,21 @@ async def wait_for_idle( going into the idle state. (e.g. useful for scaling down). When set, takes precedence over the `wait_for_units` parameter. """ + if use_new_wait_for_idle(): + await self.new_wait_for_idle( + apps=apps, + raise_on_error=raise_on_error, + raise_on_blocked=raise_on_blocked, + wait_for_active=wait_for_active, + timeout=timeout, + idle_period=idle_period, + check_freq=check_freq, + status=status, + wait_for_at_least_units=wait_for_at_least_units, + wait_for_exact_units=wait_for_exact_units, + ) + return + if wait_for_active: warnings.warn( "wait_for_active is deprecated; use status", @@ -3065,16 +3104,18 @@ async def wait_for_idle( wait_for_at_least_units if wait_for_at_least_units is not None else 1 ) - timeout = timedelta(seconds=timeout) if timeout is not None else None - idle_period = timedelta(seconds=idle_period) + timeout_ = timedelta(seconds=timeout) if timeout is not None else None + idle_period_ = timedelta(seconds=idle_period) start_time = datetime.now() - # Type check against the common error of passing a str for apps - if apps is not None and ( - not isinstance(apps, list) or any(not isinstance(o, str) for o in apps) - ): - raise JujuError(f"Expected a List[str] for apps, given {apps}") - apps = apps or self.applications + if isinstance(apps, (str, bytes, bytearray, memoryview)): + raise TypeError(f"apps must be a Iterable[str], got {apps=}") + + apps_ = list(apps or self.applications) + + if any(not isinstance(o, str) for o in apps_): + raise TypeError(f"apps must be a Iterable[str], got {apps_=}") + idle_times: dict[str, datetime] = {} units_ready: set[str] = set() # The units that are in the desired state last_log_time: datetime | None = None @@ -3110,10 +3151,10 @@ def _raise_for_status(entities: dict[str, list[str]], status: Any): # The list 'busy' is what keeps this loop going, # i.e. it'll stop when busy is empty after all the # units are scanned - busy = [] - errors = {} - blocks = {} - for app_name in apps: + busy: list[str] = [] + errors: dict[str, list[str]] = {} + blocks: dict[str, list[str]] = {} + for app_name in apps_: if app_name not in self.applications: busy.append(app_name + " (missing)") continue @@ -3192,7 +3233,7 @@ def _raise_for_status(entities: dict[str, list[str]], status: Any): now = datetime.now() idle_start = idle_times.setdefault(unit.name, now) - if now - idle_start < idle_period: + if now - idle_start < idle_period_: busy.append( f"{unit.name} [{unit.agent_status}] {unit.workload_status}: {unit.workload_status_message}" ) @@ -3205,14 +3246,84 @@ def _raise_for_status(entities: dict[str, list[str]], status: Any): _raise_for_status(blocks, "blocked") if not busy: break - busy = "\n ".join(busy) - if timeout is not None and datetime.now() - start_time > timeout: - raise asyncio.TimeoutError("Timed out waiting for model:\n" + busy) + if timeout_ is not None and datetime.now() - start_time > timeout_: + raise asyncio.TimeoutError( + "\n ".join(["Timed out waiting for model:", *busy]) + ) if last_log_time is None or datetime.now() - last_log_time > log_interval: - log.info("Waiting for model:\n " + busy) + log.info("\n ".join(["Waiting for model:", *busy])) last_log_time = datetime.now() await asyncio.sleep(check_freq) + async def new_wait_for_idle( + self, + apps: Iterable[str] | None = None, + raise_on_error: bool = True, + raise_on_blocked: bool = False, + wait_for_active: bool = False, + timeout: float | None = 10 * 60, + idle_period: float = 15, + check_freq: float = 0.5, + status: str | None = None, + wait_for_at_least_units: int | None = None, + wait_for_exact_units: int | None = None, + ) -> None: + """Wait for applications in the model to settle into an idle state. + + arguments match those of .wait_for_idle exactly. + """ + if not isinstance(wait_for_exact_units, (int, type(None))): + raise ValueError(f"Must be an int or None, got {wait_for_exact_units=}") + + if isinstance(wait_for_exact_units, int) and wait_for_exact_units < 0: + raise ValueError(f"Must be >=0, got {wait_for_exact_units=}") + + if wait_for_active: + warnings.warn( + "wait_for_active is deprecated; use status", + DeprecationWarning, + stacklevel=3, + ) + status = "active" + + wait_for_units = ( + wait_for_at_least_units if wait_for_at_least_units is not None else 1 + ) + + if isinstance(apps, (str, bytes, bytearray, memoryview)): + raise TypeError(f"apps must be a Iterable[str], got {apps=}") + + apps = frozenset(apps or self.applications) + + if any(not isinstance(o, str) for o in apps): + raise TypeError(f"apps must be a Iterable[str], got {apps=}") + + deadline = None if timeout is None else time.monotonic() + timeout + + async def status_on_demand(): + yield _idle.check( + await self.get_status(), + apps=apps, + raise_on_error=raise_on_error, + raise_on_blocked=raise_on_blocked, + status=status, + ) + + async for done in _idle.loop( + status_on_demand(), + apps=apps, + wait_for_exact_units=wait_for_exact_units, + wait_for_units=wait_for_units, + idle_period=idle_period, + ): + if done: + break + + if deadline and time.monotonic() > deadline: + raise asyncio.TimeoutError(f"Timed out after {timeout}s") + + await asyncio.sleep(check_freq) + def _create_consume_args(offer, macaroon, controller_info): """Convert a typed object that has been normalised to a overridden typed diff --git a/juju/model/_idle.py b/juju/model/_idle.py new file mode 100644 index 00000000..8f6b9a79 --- /dev/null +++ b/juju/model/_idle.py @@ -0,0 +1,221 @@ +# Copyright 2024 Canonical Ltd. +# Licensed under the Apache V2, see LICENCE file for details. +"""Implementation of Model.wait_for_idle(), analog to `juju wait_for`.""" + +from __future__ import annotations + +import logging +import time +from dataclasses import dataclass +from typing import AsyncIterable + +from ..client._definitions import ( + ApplicationStatus, + FullStatus, + MachineStatus, + UnitStatus, +) +from ..errors import JujuAgentError, JujuAppError, JujuMachineError, JujuUnitError + +logger = logging.getLogger(__name__) + + +@dataclass +class CheckStatus: + """Return type check(), represents single loop iteration.""" + + units: set[str] + """All units visible at this point.""" + ready_units: set[str] + """Units in good status (workload, agent, machine?).""" + idle_units: set[str] + """Units with stable agent status (FIXME details).""" + + +async def loop( + foo: AsyncIterable[CheckStatus | None], + *, + apps: frozenset[str], + wait_for_exact_units: int | None = None, + wait_for_units: int, + idle_period: float, +) -> AsyncIterable[bool]: + """The outer, time-dependents logic of a wait_for_idle loop.""" + idle_since: dict[str, float] = {} + + async for status in foo: + logger.warning("FIXME unit test debug %r", status) + now = time.monotonic() + + if not status: + yield False + continue + + expected_idle_since = now - idle_period + rv = True + + # FIXME there's some confusion about what a "busy" unit is + # are we ready when over the last idle_period, every time sampled: + # a. >=N units were ready (possibly different each time), or + # b. >=N units were ready each time + for name in status.units: + if name in status.ready_units: + idle_since[name] = min(now, idle_since.get(name, float("inf"))) + else: + idle_since[name] = float("inf") + + for app_name in apps: + ready_units = [ + n for n in status.ready_units if n.startswith(f"{app_name}/") + ] + if len(ready_units) < wait_for_units: + logger.warn( + "Waiting for app %r units %s >= %s", + app_name, + len(status.ready_units), + wait_for_units, + ) + rv = False + + if ( + wait_for_exact_units is not None + and len(ready_units) != wait_for_exact_units + ): + logger.warn( + "Waiting for app %r units %s == %s", + app_name, + len(ready_units), + wait_for_exact_units, + ) + rv = False + + # FIXME possible interaction between "wait_for_units" and "idle_period" + # Assume that we've got some units ready and some busy + # What are the semantics for returning True? + if busy := [n for n, t in idle_since.items() if t > expected_idle_since]: + logger.warn("Waiting for %s to be idle enough", busy) + rv = False + + yield rv + + +def check( + full_status: FullStatus, + *, + apps: frozenset[str], + raise_on_error: bool, + raise_on_blocked: bool, + status: str | None, +) -> CheckStatus | None: + """A single iteration of a wait_for_idle loop.""" + for app_name in apps: + if not full_status.applications.get(app_name): + logger.info("Waiting for app %r", app_name) + return None + + # Order of errors: + # + # Machine error (any unit of any app from apps) + # Agent error (-"-) + # Workload error (-"-) + # App error (any app from apps) + # + # Workload blocked (any unit of any app from apps) + # App blocked (any app from apps) + units: dict[str, UnitStatus] = {} + + for app_name in apps: + units.update(_app_units(full_status, app_name)) + + for unit_name, unit in units.items(): + if unit.machine: + machine = full_status.machines[unit.machine] + assert isinstance(machine, MachineStatus) + assert machine.instance_status + if machine.instance_status.status == "error" and raise_on_error: + raise JujuMachineError( + f"{unit_name!r} machine {unit.machine!r} has errored: {machine.instance_status.info!r}" + ) + + for unit_name, unit in units.items(): + assert unit.agent_status + if unit.agent_status.status == "error" and raise_on_error: + raise JujuAgentError( + f"{unit_name!r} agent has errored: {unit.agent_status.info!r}" + ) + + for unit_name, unit in units.items(): + assert unit.workload_status + if unit.workload_status.status == "error" and raise_on_error: + raise JujuUnitError( + f"{unit_name!r} workload has errored: {unit.workload_status.info!r}" + ) + + for app_name in apps: + app = full_status.applications[app_name] + assert isinstance(app, ApplicationStatus) + assert app.status + if app.status.status == "error" and raise_on_error: + raise JujuAppError(f"{app_name!r} has errored: {app.status.info!r}") + + for unit_name, unit in units.items(): + assert unit.workload_status + if unit.workload_status.status == "blocked" and raise_on_blocked: + raise JujuUnitError( + f"{unit_name!r} workload is blocked: {unit.workload_status.info!r}" + ) + + for app_name in apps: + app = full_status.applications[app_name] + assert isinstance(app, ApplicationStatus) + assert app.status + if app.status.status == "blocked" and raise_on_blocked: + raise JujuAppError(f"{app_name!r} is blocked: {app.status.info!r}") + + rv = CheckStatus(set(), set(), set()) + + for app_name in apps: + ready_units = [] + app = full_status.applications[app_name] + assert isinstance(app, ApplicationStatus) + for unit_name, unit in _app_units(full_status, app_name).items(): + rv.units.add(unit_name) + assert unit.agent_status + assert unit.workload_status + + if unit.agent_status.status != "idle": + continue + if status and unit.workload_status.status != status: + continue + + ready_units.append(unit) + rv.ready_units.add(unit_name) + + # FIXME + # rv.idle_units -- depends on agent status only, not workload status + return rv + + +def _app_units(full_status: FullStatus, app_name: str) -> dict[str, UnitStatus]: + """Fish out the app's units' status from a FullStatus response.""" + rv: dict[str, UnitStatus] = {} + app = full_status.applications[app_name] + assert isinstance(app, ApplicationStatus) + + if app.subordinate_to: + parent_name = app.subordinate_to[0] + parent = full_status.applications[parent_name] + assert isinstance(parent, ApplicationStatus) + for parent_unit in parent.units.values(): + assert isinstance(parent_unit, UnitStatus) + for name, unit in parent_unit.subordinates.items(): + if not name.startswith(f"{app_name}/"): + continue + assert isinstance(unit, UnitStatus) + rv[name] = unit + else: + for name, unit in app.units.items(): + assert isinstance(unit, UnitStatus) + rv[name] = unit + + return rv diff --git a/juju/unit.py b/juju/unit.py index 4cb8e259..64e14ac6 100644 --- a/juju/unit.py +++ b/juju/unit.py @@ -15,7 +15,9 @@ class Unit(model.ModelEntity): - name: str + @property + def name(self) -> str: + return self.entity_id @property def agent_status(self): diff --git a/juju/version.py b/juju/version.py index b3e36f02..e8d250a6 100644 --- a/juju/version.py +++ b/juju/version.py @@ -6,4 +6,4 @@ DEFAULT_ARCHITECTURE = "amd64" -CLIENT_VERSION = "3.6.0.0" +CLIENT_VERSION = "3.6.1.0rc2" diff --git a/pyproject.toml b/pyproject.toml index b3c2d427..6947ad82 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "juju" -version = "3.6.0.0" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION +version = "3.6.1.0rc2" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION description = "Python library for Juju" readme = "docs/readme.rst" license = { file = "LICENSE" } @@ -42,6 +42,7 @@ dev = [ "pytest", "pytest-asyncio", "Twine", + "freezegun", ] docs = [ "sphinx==5.3.0", @@ -226,8 +227,9 @@ ignore = [ # These are tentative include = ["**/*.py"] pythonVersion = "3.8.6" -typeCheckingMode = "strict" -useLibraryCodeForTypes = true +# FIXME understand the difference +#typeCheckingMode = "strict" +#useLibraryCodeForTypes = true reportGeneralTypeIssues = true [tool.pytest.ini_options] diff --git a/setup.py b/setup.py index 93825375..57a713de 100644 --- a/setup.py +++ b/setup.py @@ -40,6 +40,7 @@ "pytest", "pytest-asyncio", "Twine", + "freezegun", ] }, include_package_data=True, diff --git a/tests/unit/data/fullstatus.json b/tests/unit/data/fullstatus.json new file mode 100644 index 00000000..d470a8c1 --- /dev/null +++ b/tests/unit/data/fullstatus.json @@ -0,0 +1,322 @@ +{ + "request-id": 7, + "response": { + "applications": { + "grafana-agent-k8s": { + "base": { + "channel": "22.04/stable", + "name": "ubuntu" + }, + "can-upgrade-to": "", + "charm": "ch:arm64/jammy/grafana-agent-k8s-75", + "charm-channel": "latest/stable", + "charm-profile": "", + "charm-version": "", + "endpoint-bindings": { + "": "alpha", + "certificates": "alpha", + "grafana-cloud-config": "alpha", + "grafana-dashboards-consumer": "alpha", + "grafana-dashboards-provider": "alpha", + "logging-consumer": "alpha", + "logging-provider": "alpha", + "metrics-endpoint": "alpha", + "peers": "alpha", + "receive-ca-cert": "alpha", + "send-remote-write": "alpha", + "tracing": "alpha" + }, + "exposed": false, + "int": 1, + "life": "", + "meter-statuses": null, + "provider-id": "4ecc75be-f038-4452-b1af-640d1b46f1c6", + "public-address": "10.152.183.55", + "relations": { + "peers": [ + "grafana-agent-k8s" + ] + }, + "status": { + "data": {}, + "info": "installing agent", + "kind": "", + "life": "", + "since": "2024-09-30T07:44:15.63582531Z", + "status": "waiting", + "version": "" + }, + "subordinate-to": [], + "units": { + "grafana-agent-k8s/0": { + "address": "10.1.121.164", + "agent-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T07:44:15.469295423Z", + "status": "idle", + "version": "3.5.1" + }, + "charm": "", + "leader": true, + "machine": "", + "opened-ports": [], + "provider-id": "grafana-agent-k8s-0", + "public-address": "", + "subordinates": null, + "workload-status": { + "data": {}, + "info": "Missing incoming (\"requires\") relation: metrics-endpoint|logging-provider|grafana-dashboards-consumer", + "kind": "", + "life": "", + "since": "2024-09-30T07:43:41.649319444Z", + "status": "blocked", + "version": "" + }, + "workload-version": "0.35.2" + } + }, + "workload-version": "0.35.2" + }, + "hexanator": { + "base": { + "channel": "24.04/stable", + "name": "ubuntu" + }, + "can-upgrade-to": "", + "charm": "local:noble/hexanator-1", + "charm-profile": "", + "charm-version": "", + "endpoint-bindings": { + "": "alpha", + "ingress": "alpha", + "rate-limit": "alpha" + }, + "exposed": false, + "int": 1, + "life": "", + "meter-statuses": null, + "provider-id": "b5efccf2-5a15-41a0-af0f-689a8d93a129", + "public-address": "10.152.183.113", + "relations": {}, + "status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T00:12:47.878239549Z", + "status": "active", + "version": "" + }, + "subordinate-to": [], + "units": { + "hexanator/0": { + "address": "10.1.121.184", + "agent-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T00:13:16.731257044Z", + "status": "idle", + "version": "3.5.1" + }, + "charm": "", + "leader": true, + "machine": "", + "opened-ports": [], + "provider-id": "hexanator-0", + "public-address": "", + "subordinates": null, + "workload-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T00:12:47.878239549Z", + "status": "active", + "version": "" + }, + "workload-version": "" + } + }, + "workload-version": "" + }, + "mysql-test-app": { + "base": { + "channel": "22.04/stable", + "name": "ubuntu" + }, + "can-upgrade-to": "", + "charm": "ch:arm64/jammy/mysql-test-app-62", + "charm-channel": "latest/edge", + "charm-profile": "", + "charm-version": "", + "endpoint-bindings": { + "": "alpha", + "application-peers": "alpha", + "database": "alpha", + "mysql": "alpha" + }, + "exposed": false, + "int": 2, + "life": "", + "meter-statuses": null, + "provider-id": "4338786a-a337-4779-820d-679a59ba1665", + "public-address": "10.152.183.118", + "relations": { + "application-peers": [ + "mysql-test-app" + ] + }, + "status": { + "data": {}, + "info": "installing agent", + "kind": "", + "life": "", + "since": "2024-09-30T07:48:25.106109123Z", + "status": "waiting", + "version": "" + }, + "subordinate-to": [], + "units": { + "mysql-test-app/0": { + "address": "10.1.121.142", + "agent-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-10-01T00:15:03.216904329Z", + "status": "idle", + "version": "3.5.1" + }, + "charm": "", + "leader": true, + "machine": "", + "opened-ports": [], + "provider-id": "mysql-test-app-0", + "public-address": "", + "subordinates": null, + "workload-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T07:47:54.212959856Z", + "status": "waiting", + "version": "" + }, + "workload-version": "0.0.2" + }, + "mysql-test-app/1": { + "address": "10.1.121.190", + "agent-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T23:49:39.923901864Z", + "status": "idle", + "version": "3.5.1" + }, + "charm": "", + "machine": "", + "opened-ports": [], + "provider-id": "mysql-test-app-1", + "public-address": "", + "subordinates": null, + "workload-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T07:47:54.211414881Z", + "status": "waiting", + "version": "" + }, + "workload-version": "0.0.2" + } + }, + "workload-version": "0.0.2" + } + }, + "branches": {}, + "controller-timestamp": "2024-10-01T07:25:22.51380313Z", + "machines": {}, + "model": { + "available-version": "", + "cloud-tag": "cloud-microk8s", + "meter-status": { + "color": "", + "message": "" + }, + "model-status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-27T08:21:45.368693216Z", + "status": "available", + "version": "" + }, + "name": "testm", + "region": "localhost", + "sla": "unsupported", + "type": "caas", + "version": "3.5.1" + }, + "offers": {}, + "relations": [ + { + "endpoints": [ + { + "application": "grafana-agent-k8s", + "name": "peers", + "role": "peer", + "subordinate": false + } + ], + "id": 0, + "interface": "grafana_agent_replica", + "key": "grafana-agent-k8s:peers", + "scope": "global", + "status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T07:43:31.018463595Z", + "status": "joined", + "version": "" + } + }, + { + "endpoints": [ + { + "application": "mysql-test-app", + "name": "application-peers", + "role": "peer", + "subordinate": false + } + ], + "id": 1, + "interface": "application-peers", + "key": "mysql-test-app:application-peers", + "scope": "global", + "status": { + "data": {}, + "info": "", + "kind": "", + "life": "", + "since": "2024-09-30T07:47:52.823202648Z", + "status": "joined", + "version": "" + } + } + ], + "remote-applications": {} + } +} diff --git a/tests/unit/data/subordinate-fullstatus.json b/tests/unit/data/subordinate-fullstatus.json new file mode 100644 index 00000000..12c66d70 --- /dev/null +++ b/tests/unit/data/subordinate-fullstatus.json @@ -0,0 +1,254 @@ +{ + "request-id": 2618, + "response": { + "model": { + "name": "test-7442-test-subordinate-units-8b20", + "type": "iaas", + "cloud-tag": "cloud-localhost", + "region": "localhost", + "version": "3.6.0", + "available-version": "", + "model-status": { + "status": "available", + "info": "", + "data": {}, + "since": "2024-12-04T01:41:47.4040454Z", + "kind": "", + "version": "", + "life": "" + }, + "meter-status": {"color": "", "message": ""}, + "sla": "unsupported" + }, + "machines": { + "0": { + "agent-status": { + "status": "started", + "info": "", + "data": {}, + "since": "2024-12-04T01:43:51.558449988Z", + "kind": "", + "version": "3.6.0", + "life": "" + }, + "instance-status": { + "status": "running", + "info": "Running", + "data": {}, + "since": "2024-12-04T01:42:38.710685177Z", + "kind": "", + "version": "", + "life": "" + }, + "modification-status": { + "status": "applied", + "info": "", + "data": {}, + "since": "2024-12-04T01:42:26.414748546Z", + "kind": "", + "version": "", + "life": "" + }, + "hostname": "juju-eb2c2c-0", + "dns-name": "10.149.76.219", + "ip-addresses": ["10.149.76.219"], + "instance-id": "juju-eb2c2c-0", + "display-name": "", + "base": {"name": "ubuntu", "channel": "22.04/stable"}, + "id": "0", + "network-interfaces": { + "eth0": { + "ip-addresses": ["10.149.76.219"], + "mac-address": "00:16:3e:06:13:5f", + "gateway": "10.149.76.1", + "space": "alpha", + "is-up": true + } + }, + "containers": {}, + "constraints": "arch=amd64", + "hardware": "arch=amd64 cores=0 mem=0M virt-type=container", + "jobs": ["JobHostUnits"], + "has-vote": false, + "wants-vote": false + } + }, + "applications": { + "ntp": { + "charm": "ch:amd64/ntp-50", + "charm-version": "cs-ntp-team-ntp-4-171-g669ff59", + "charm-profile": "", + "charm-channel": "latest/stable", + "charm-rev": 50, + "base": {"name": "ubuntu", "channel": "20.04/stable"}, + "exposed": false, + "life": "", + "relations": {"juju-info": ["ubuntu"], "ntp-peers": ["ntp"]}, + "can-upgrade-to": "", + "subordinate-to": ["ubuntu"], + "units": null, + "meter-statuses": null, + "status": { + "status": "active", + "info": "chrony: Ready, Failed to disable Hyper-V host sync", + "data": {}, + "since": "2024-12-04T01:44:40.346093963Z", + "kind": "", + "version": "", + "life": "" + }, + "workload-version": "4.2", + "endpoint-bindings": { + "": "alpha", + "juju-info": "alpha", + "master": "alpha", + "nrpe-external-master": "alpha", + "ntp-peers": "alpha", + "ntpmaster": "alpha" + }, + "public-address": "" + }, + "ubuntu": { + "charm": "ch:amd64/ubuntu-25", + "charm-version": "", + "charm-profile": "", + "charm-channel": "latest/stable", + "charm-rev": 25, + "base": {"name": "ubuntu", "channel": "22.04/stable"}, + "exposed": false, + "life": "", + "relations": {"juju-info": ["ntp"]}, + "can-upgrade-to": "", + "subordinate-to": [], + "units": { + "ubuntu/0": { + "agent-status": { + "status": "idle", + "info": "", + "data": {}, + "since": "2024-12-04T01:44:44.342778729Z", + "kind": "", + "version": "3.6.0", + "life": "" + }, + "workload-status": { + "status": "active", + "info": "", + "data": {}, + "since": "2024-12-04T01:43:53.391031729Z", + "kind": "", + "version": "", + "life": "" + }, + "workload-version": "22.04", + "machine": "0", + "opened-ports": null, + "public-address": "10.149.76.219", + "charm": "", + "subordinates": { + "ntp/0": { + "agent-status": { + "status": "idle", + "info": "", + "data": {}, + "since": "2024-12-04T01:44:47.418242454Z", + "kind": "", + "version": "3.6.0", + "life": "" + }, + "workload-status": { + "status": "active", + "info": "chrony: Ready, Failed to disable Hyper-V host sync", + "data": {}, + "since": "2024-12-04T01:44:40.346093963Z", + "kind": "", + "version": "", + "life": "" + }, + "workload-version": "4.2", + "machine": "", + "opened-ports": ["123/udp"], + "public-address": "10.149.76.219", + "charm": "", + "subordinates": null, + "leader": true + } + }, + "leader": true + } + }, + "meter-statuses": null, + "status": { + "status": "active", + "info": "", + "data": {}, + "since": "2024-12-04T01:43:53.391031729Z", + "kind": "", + "version": "", + "life": "" + }, + "workload-version": "22.04", + "endpoint-bindings": null, + "public-address": "" + } + }, + "remote-applications": {}, + "offers": {}, + "relations": [ + { + "id": 0, + "key": "ntp:ntp-peers", + "interface": "ntp", + "scope": "global", + "endpoints": [ + { + "application": "ntp", + "name": "ntp-peers", + "role": "peer", + "subordinate": false + } + ], + "status": { + "status": "joined", + "info": "", + "data": {}, + "since": "2024-12-04T01:44:43.940973679Z", + "kind": "", + "version": "", + "life": "" + } + }, + { + "id": 1, + "key": "ntp:juju-info ubuntu:juju-info", + "interface": "juju-info", + "scope": "container", + "endpoints": [ + { + "application": "ubuntu", + "name": "juju-info", + "role": "provider", + "subordinate": false + }, + { + "application": "ntp", + "name": "juju-info", + "role": "requirer", + "subordinate": true + } + ], + "status": { + "status": "joined", + "info": "", + "data": {}, + "since": "2024-12-04T01:43:53.72325443Z", + "kind": "", + "version": "", + "life": "" + } + } + ], + "controller-timestamp": "2024-12-04T02:01:53.569630593Z", + "branches": {} + } +} diff --git a/tests/unit/test_idle_check.py b/tests/unit/test_idle_check.py new file mode 100644 index 00000000..8ed89e4f --- /dev/null +++ b/tests/unit/test_idle_check.py @@ -0,0 +1,273 @@ +# Copyright 2024 Canonical Ltd. +# Licensed under the Apache V2, see LICENCE file for details. +from __future__ import annotations + +import copy +import json +from typing import Any + +import pytest + +from juju.client._definitions import ( + FullStatus, +) +from juju.client.facade import _convert_response +from juju.errors import JujuAgentError, JujuAppError, JujuMachineError, JujuUnitError +from juju.model._idle import CheckStatus, check + + +def test_check_status(full_status: FullStatus, kwargs): + status = check(full_status, **kwargs) + assert status == CheckStatus( + { + "grafana-agent-k8s/0", + "hexanator/0", + "mysql-test-app/0", + "mysql-test-app/1", + }, + { + "grafana-agent-k8s/0", + "hexanator/0", + "mysql-test-app/0", + "mysql-test-app/1", + }, + set(), + ) + + +def test_check_status_missing_app(full_status: FullStatus, kwargs): + kwargs["apps"] = ["missing", "hexanator"] + status = check(full_status, **kwargs) + assert status is None + + +def test_check_status_is_selective(full_status: FullStatus, kwargs): + kwargs["apps"] = ["hexanator"] + status = check(full_status, **kwargs) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + + +def test_no_apps(full_status: FullStatus, kwargs): + kwargs["apps"] = [] + status = check(full_status, **kwargs) + assert status == CheckStatus(set(), set(), set()) + + +def test_missing_app(full_status: FullStatus, kwargs): + kwargs["apps"] = ["missing"] + status = check(full_status, **kwargs) + assert status is None + + +def test_no_units(response: dict[str, Any], kwargs): + response["response"]["applications"]["hexanator"]["units"].clear() + kwargs["apps"] = ["hexanator"] + status = check(convert(response), **kwargs) + assert status == CheckStatus(set(), set(), set()) + + +def test_app_error(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["status"]["status"] = "error" + app["status"]["info"] = "big problem" + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_error"] = True + + with pytest.raises(JujuAppError) as e: + check(convert(response), **kwargs) + + assert "big problem" in str(e) + + +def test_exact_count(response: dict[str, Any], kwargs): + units = response["response"]["applications"]["hexanator"]["units"] + units["hexanator/1"] = units["hexanator/0"] + + kwargs["apps"] = ["hexanator"] + + status = check(convert(response), **kwargs) + assert status == CheckStatus( + {"hexanator/0", "hexanator/1"}, {"hexanator/0", "hexanator/1"}, set() + ) + + +def test_ready_units(full_status: FullStatus, kwargs): + kwargs["apps"] = ["mysql-test-app"] + status = check(full_status, **kwargs) + assert status == CheckStatus( + {"mysql-test-app/0", "mysql-test-app/1"}, + {"mysql-test-app/0", "mysql-test-app/1"}, + set(), + ) + + +def test_active_units(full_status: FullStatus, kwargs): + kwargs["apps"] = ["mysql-test-app"] + kwargs["status"] = "active" + status = check(full_status, **kwargs) + assert status == CheckStatus({"mysql-test-app/0", "mysql-test-app/1"}, set(), set()) + + +def test_ready_unit_requires_idle_agent(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/1"] = copy.deepcopy(app["units"]["hexanator/0"]) + app["units"]["hexanator/1"]["agent-status"]["status"] = "some-other" + + kwargs["apps"] = ["hexanator"] + kwargs["status"] = "active" + + status = check(convert(response), **kwargs) + assert status + assert status.units == {"hexanator/0", "hexanator/1"} + assert status.ready_units == {"hexanator/0"} + + +def test_ready_unit_requires_workload_status(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/1"] = copy.deepcopy(app["units"]["hexanator/0"]) + app["units"]["hexanator/1"]["workload-status"]["status"] = "some-other" + + kwargs["apps"] = ["hexanator"] + kwargs["status"] = "active" + + status = check(convert(response), **kwargs) + assert status == CheckStatus({"hexanator/0", "hexanator/1"}, {"hexanator/0"}, set()) + + +def test_agent_error(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/0"]["agent-status"]["status"] = "error" + app["units"]["hexanator/0"]["agent-status"]["info"] = "agent problem" + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_error"] = True + + with pytest.raises(JujuAgentError) as e: + check(convert(response), **kwargs) + + assert "hexanator/0" in str(e) + assert "agent problem" in str(e) + + +def test_workload_error(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/0"]["workload-status"]["status"] = "error" + app["units"]["hexanator/0"]["workload-status"]["info"] = "workload problem" + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_error"] = True + + with pytest.raises(JujuUnitError) as e: + check(convert(response), **kwargs) + + assert "hexanator/0" in str(e) + assert "workload problem" in str(e) + + +def test_machine_ok(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/0"]["machine"] = "42" + # https://github.com/dimaqq/juju-schema-analysis/blob/main/schemas-juju-3.5.4.txt#L3611-L3674 + response["response"]["machines"] = { + "42": { + "instance-status": { + "status": "running", + "info": "RUNNING", + }, + }, + } + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_error"] = True + + status = check(convert(response), **kwargs) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + + +def test_machine_error(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/0"]["machine"] = "42" + response["response"]["machines"] = { + "42": { + "instance-status": { + "status": "error", + "info": "Battery low. Try a potato?", + }, + }, + } + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_error"] = True + + with pytest.raises(JujuMachineError) as e: + check(convert(response), **kwargs) + + assert "potato" in str(e) + + +def test_app_blocked(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["status"]["status"] = "blocked" + app["status"]["info"] = "big problem" + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_blocked"] = True + + with pytest.raises(JujuAppError) as e: + check(convert(response), **kwargs) + + assert "big problem" in str(e) + + +def test_unit_blocked(response: dict[str, Any], kwargs): + app = response["response"]["applications"]["hexanator"] + app["units"]["hexanator/0"]["workload-status"]["status"] = "blocked" + app["units"]["hexanator/0"]["workload-status"]["info"] = "small problem" + + kwargs["apps"] = ["hexanator"] + kwargs["raise_on_blocked"] = True + + with pytest.raises(JujuUnitError) as e: + check(convert(response), **kwargs) + + assert "small problem" in str(e) + + +def test_maintenance(response: dict[str, Any], kwargs): + """Taken from nginx-ingress-integrator-operator integration tests.""" + app = response["response"]["applications"]["hexanator"] + app["status"]["status"] = "maintenance" + app["units"]["hexanator/0"]["workload-status"]["status"] = "maintenance" + + kwargs["apps"] = ["hexanator"] + kwargs["status"] = "maintenance" + + status = check(convert(response), **kwargs) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + + +@pytest.fixture +def kwargs() -> dict[str, Any]: + return dict( + apps=["hexanator", "grafana-agent-k8s", "mysql-test-app"], + raise_on_error=False, + raise_on_blocked=False, + status=None, + ) + + +@pytest.fixture +def response(pytestconfig: pytest.Config) -> dict[str, Any]: + return json.loads( + (pytestconfig.rootpath / "tests/unit/data/fullstatus.json").read_text() + ) + + +def convert(data: dict[str, Any]) -> FullStatus: + return _convert_response(data, cls=FullStatus) + + +@pytest.fixture +def full_status(response) -> FullStatus: + return convert(response) diff --git a/tests/unit/test_idle_check_subordinate.py b/tests/unit/test_idle_check_subordinate.py new file mode 100644 index 00000000..149e468b --- /dev/null +++ b/tests/unit/test_idle_check_subordinate.py @@ -0,0 +1,57 @@ +# Copyright 2024 Canonical Ltd. +# Licensed under the Apache V2, see LICENCE file for details. +from __future__ import annotations + +import json +from typing import Any + +import pytest + +from juju.client._definitions import ( + FullStatus, +) +from juju.client.facade import _convert_response +from juju.model._idle import CheckStatus, check + + +def test_subordinate_apps(response: dict[str, Any], kwargs): + status = check(convert(response), **kwargs) + assert status == CheckStatus({"ntp/0", "ubuntu/0"}, {"ntp/0", "ubuntu/0"}, set()) + + +def test_subordinate_is_selective(response, kwargs): + subordinates = response["response"]["applications"]["ubuntu"]["units"]["ubuntu/0"][ + "subordinates" + ] + subordinates["some-other/0"] = subordinates["ntp/0"] + status = check(convert(response), **kwargs) + assert status == CheckStatus({"ntp/0", "ubuntu/0"}, {"ntp/0", "ubuntu/0"}, set()) + + +@pytest.fixture +def kwargs() -> dict[str, Any]: + return dict( + apps=["ntp", "ubuntu"], + raise_on_error=False, + raise_on_blocked=False, + status=None, + ) + + +@pytest.fixture +def response(pytestconfig: pytest.Config) -> dict[str, Any]: + """Juju rpc response JSON to a FullStatus call.""" + return json.loads( + ( + pytestconfig.rootpath / "tests/unit/data/subordinate-fullstatus.json" + ).read_text() + ) + + +@pytest.fixture +def subordinate_status(response) -> FullStatus: + return convert(response) + + +def convert(data: dict[str, Any]) -> FullStatus: + return _convert_response(data, cls=FullStatus) diff --git a/tests/unit/test_idle_loop.py b/tests/unit/test_idle_loop.py new file mode 100644 index 00000000..34a8c672 --- /dev/null +++ b/tests/unit/test_idle_loop.py @@ -0,0 +1,61 @@ +# Copyright 2024 Canonical Ltd. +# Licensed under the Apache V2, see LICENCE file for details. +from __future__ import annotations + +import pytest +from freezegun import freeze_time + +from juju.model._idle import CheckStatus, loop + + +# Missing tests +# +# FIXME hexanator idle period 1 +# FIXME workload maintenance, idle period 0 +# FIXME test exact count == 2 +# FIXME test exact count != 2 (1, 3) +# FIXME exact count vs wait_for_units +# FIXME expected idle 1s below +# FIXME idle period 1 +# FIXME sending status=None, meaning some apps are still missing +# +@pytest.mark.xfail(reason="FIXME I misunderstood what 'idle' means") +async def test_at_least_units(): + async def checks(): + yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0"}, set()) + yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1"}, set()) + yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1", "u/2"}, set()) + + with freeze_time(): + assert [ + v + async for v in loop( + checks(), + apps=frozenset(["u"]), + wait_for_units=2, + idle_period=0, + ) + ] == [False, True, True] + + +async def test_ping_pong(): + good = CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + bad = CheckStatus({"hexanator/0"}, set(), set()) + + async def checks(): + with freeze_time() as clock: + for _ in range(3): + yield good + clock.tick(10) + yield bad + clock.tick(10) + + assert [ + v + async for v in loop( + checks(), + apps=frozenset(["hexanator"]), + wait_for_units=1, + idle_period=15, + ) + ] == [False] * 6 diff --git a/tests/unit/test_model.py b/tests/unit/test_model.py index 09104321..cde663a5 100644 --- a/tests/unit/test_model.py +++ b/tests/unit/test_model.py @@ -11,7 +11,7 @@ from juju.application import Application from juju.client.jujudata import FileJujuData -from juju.errors import JujuConnectionError, JujuError +from juju.errors import JujuConnectionError from juju.model import Model @@ -251,19 +251,19 @@ async def test_no_args(self): # no apps so should return right away await m.wait_for_idle(wait_for_active=True) - async def test_apps_no_lst(self): + async def test_apps_type(self): m = Model() - with self.assertRaises(JujuError): + with self.assertRaises(TypeError): # apps arg has to be a List[str] await m.wait_for_idle(apps="should-be-list") - with self.assertRaises(JujuError): + with self.assertRaises(TypeError): # apps arg has to be a List[str] - await m.wait_for_idle(apps=3) + await m.wait_for_idle(apps=3) # type: ignore - with self.assertRaises(JujuError): + with self.assertRaises(TypeError): # apps arg has to be a List[str] - await m.wait_for_idle(apps=[3]) + await m.wait_for_idle(apps=[3]) # type: ignore async def test_timeout(self): m = Model() @@ -271,7 +271,8 @@ async def test_timeout(self): # no apps so should timeout after timeout period await m.wait_for_idle(apps=["nonexisting_app"]) self.assertEqual( - str(cm.exception), "Timed out waiting for model:\nnonexisting_app (missing)" + str(cm.exception), + "Timed out waiting for model:\n nonexisting_app (missing)", ) @pytest.mark.wait_for_idle diff --git a/tests/unit/test_unit.py b/tests/unit/test_unit.py index 432b5fd1..4023a170 100644 --- a/tests/unit/test_unit.py +++ b/tests/unit/test_unit.py @@ -81,7 +81,7 @@ async def test_unit_is_leader(mock_cf): client_facade.FullStatus = mock.AsyncMock(return_value=status) unit = Unit("test", model) - unit.name = test["unit"] + unit.entity_id = test["unit"] rval = await unit.is_leader_from_status() assert rval == test["rval"] diff --git a/tox.ini b/tox.ini index d284673b..2d676029 100644 --- a/tox.ini +++ b/tox.ini @@ -17,6 +17,7 @@ passenv = HOME TEST_AGENTS LXD_DIR + JUJU_NEW_WAIT_FOR_IDLE [testenv:docs] deps = From 065d3deb0d6d07d5b49e4bd2f5c47139ad14d7a7 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 12:09:58 +0900 Subject: [PATCH 2/8] chore: remplement best guess for idle timer --- juju/model/__init__.py | 4 ++- juju/model/_idle.py | 42 ++++++++++------------- juju/version.py | 2 +- pyproject.toml | 2 +- tests/unit/test_idle_check.py | 40 ++++++++++----------- tests/unit/test_idle_check_subordinate.py | 12 +++++-- 6 files changed, 51 insertions(+), 51 deletions(-) diff --git a/juju/model/__init__.py b/juju/model/__init__.py index 0063fbb6..f40738b1 100644 --- a/juju/model/__init__.py +++ b/juju/model/__init__.py @@ -3298,7 +3298,8 @@ async def new_wait_for_idle( if any(not isinstance(o, str) for o in apps): raise TypeError(f"apps must be a Iterable[str], got {apps=}") - deadline = None if timeout is None else time.monotonic() + timeout + started = time.monotonic() + deadline = None if timeout is None else started + timeout async def status_on_demand(): yield _idle.check( @@ -3316,6 +3317,7 @@ async def status_on_demand(): wait_for_units=wait_for_units, idle_period=idle_period, ): + logger.info(f"wait_for_idle start{time.monotonic() - started:+.1f} {done=}") if done: break diff --git a/juju/model/_idle.py b/juju/model/_idle.py index 8f6b9a79..6737fe12 100644 --- a/juju/model/_idle.py +++ b/juju/model/_idle.py @@ -44,7 +44,7 @@ async def loop( idle_since: dict[str, float] = {} async for status in foo: - logger.warning("FIXME unit test debug %r", status) + logger.info("wait_for_idle iteration %s", status) now = time.monotonic() if not status: @@ -52,51 +52,50 @@ async def loop( continue expected_idle_since = now - idle_period - rv = True # FIXME there's some confusion about what a "busy" unit is # are we ready when over the last idle_period, every time sampled: # a. >=N units were ready (possibly different each time), or # b. >=N units were ready each time for name in status.units: - if name in status.ready_units: + if name in status.idle_units: idle_since[name] = min(now, idle_since.get(name, float("inf"))) else: idle_since[name] = float("inf") + if busy := {n for n, t in idle_since.items() if t > expected_idle_since}: + logger.info("Waiting for units to be idle enough: %s", busy) + yield False + continue + for app_name in apps: ready_units = [ n for n in status.ready_units if n.startswith(f"{app_name}/") ] if len(ready_units) < wait_for_units: - logger.warn( + logger.info( "Waiting for app %r units %s >= %s", app_name, len(status.ready_units), wait_for_units, ) - rv = False + yield False + continue if ( wait_for_exact_units is not None and len(ready_units) != wait_for_exact_units ): - logger.warn( + logger.info( "Waiting for app %r units %s == %s", app_name, len(ready_units), wait_for_exact_units, ) - rv = False - - # FIXME possible interaction between "wait_for_units" and "idle_period" - # Assume that we've got some units ready and some busy - # What are the semantics for returning True? - if busy := [n for n, t in idle_since.items() if t > expected_idle_since]: - logger.warn("Waiting for %s to be idle enough", busy) - rv = False + yield False + continue - yield rv + yield True def check( @@ -175,7 +174,6 @@ def check( rv = CheckStatus(set(), set(), set()) for app_name in apps: - ready_units = [] app = full_status.applications[app_name] assert isinstance(app, ApplicationStatus) for unit_name, unit in _app_units(full_status, app_name).items(): @@ -183,16 +181,12 @@ def check( assert unit.agent_status assert unit.workload_status - if unit.agent_status.status != "idle": - continue - if status and unit.workload_status.status != status: - continue + if unit.agent_status.status == "idle": + rv.idle_units.add(unit_name) - ready_units.append(unit) - rv.ready_units.add(unit_name) + if not status or unit.workload_status.status == status: + rv.ready_units.add(unit_name) - # FIXME - # rv.idle_units -- depends on agent status only, not workload status return rv diff --git a/juju/version.py b/juju/version.py index e8d250a6..86177ffd 100644 --- a/juju/version.py +++ b/juju/version.py @@ -6,4 +6,4 @@ DEFAULT_ARCHITECTURE = "amd64" -CLIENT_VERSION = "3.6.1.0rc2" +CLIENT_VERSION = "3.6.1.0rc3" diff --git a/pyproject.toml b/pyproject.toml index 6947ad82..d2eec1cc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "juju" -version = "3.6.1.0rc2" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION +version = "3.6.1.0rc3" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION description = "Python library for Juju" readme = "docs/readme.rst" license = { file = "LICENSE" } diff --git a/tests/unit/test_idle_check.py b/tests/unit/test_idle_check.py index 8ed89e4f..bba2cee4 100644 --- a/tests/unit/test_idle_check.py +++ b/tests/unit/test_idle_check.py @@ -5,6 +5,7 @@ import copy import json from typing import Any +from unittest.mock import ANY import pytest @@ -31,7 +32,12 @@ def test_check_status(full_status: FullStatus, kwargs): "mysql-test-app/0", "mysql-test-app/1", }, - set(), + { + "grafana-agent-k8s/0", + "hexanator/0", + "mysql-test-app/0", + "mysql-test-app/1", + }, ) @@ -44,7 +50,7 @@ def test_check_status_missing_app(full_status: FullStatus, kwargs): def test_check_status_is_selective(full_status: FullStatus, kwargs): kwargs["apps"] = ["hexanator"] status = check(full_status, **kwargs) - assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, {"hexanator/0"}) def test_no_apps(full_status: FullStatus, kwargs): @@ -80,25 +86,13 @@ def test_app_error(response: dict[str, Any], kwargs): assert "big problem" in str(e) -def test_exact_count(response: dict[str, Any], kwargs): - units = response["response"]["applications"]["hexanator"]["units"] - units["hexanator/1"] = units["hexanator/0"] - - kwargs["apps"] = ["hexanator"] - - status = check(convert(response), **kwargs) - assert status == CheckStatus( - {"hexanator/0", "hexanator/1"}, {"hexanator/0", "hexanator/1"}, set() - ) - - def test_ready_units(full_status: FullStatus, kwargs): kwargs["apps"] = ["mysql-test-app"] status = check(full_status, **kwargs) assert status == CheckStatus( {"mysql-test-app/0", "mysql-test-app/1"}, {"mysql-test-app/0", "mysql-test-app/1"}, - set(), + {"mysql-test-app/0", "mysql-test-app/1"}, ) @@ -106,7 +100,7 @@ def test_active_units(full_status: FullStatus, kwargs): kwargs["apps"] = ["mysql-test-app"] kwargs["status"] = "active" status = check(full_status, **kwargs) - assert status == CheckStatus({"mysql-test-app/0", "mysql-test-app/1"}, set(), set()) + assert status == CheckStatus({"mysql-test-app/0", "mysql-test-app/1"}, set(), ANY) def test_ready_unit_requires_idle_agent(response: dict[str, Any], kwargs): @@ -118,9 +112,11 @@ def test_ready_unit_requires_idle_agent(response: dict[str, Any], kwargs): kwargs["status"] = "active" status = check(convert(response), **kwargs) - assert status - assert status.units == {"hexanator/0", "hexanator/1"} - assert status.ready_units == {"hexanator/0"} + assert status == CheckStatus( + {"hexanator/0", "hexanator/1"}, + {"hexanator/0", "hexanator/1"}, + {"hexanator/0"}, + ) def test_ready_unit_requires_workload_status(response: dict[str, Any], kwargs): @@ -132,7 +128,7 @@ def test_ready_unit_requires_workload_status(response: dict[str, Any], kwargs): kwargs["status"] = "active" status = check(convert(response), **kwargs) - assert status == CheckStatus({"hexanator/0", "hexanator/1"}, {"hexanator/0"}, set()) + assert status == CheckStatus({"hexanator/0", "hexanator/1"}, {"hexanator/0"}, ANY) def test_agent_error(response: dict[str, Any], kwargs): @@ -182,7 +178,7 @@ def test_machine_ok(response: dict[str, Any], kwargs): kwargs["raise_on_error"] = True status = check(convert(response), **kwargs) - assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, ANY) def test_machine_error(response: dict[str, Any], kwargs): @@ -244,7 +240,7 @@ def test_maintenance(response: dict[str, Any], kwargs): kwargs["status"] = "maintenance" status = check(convert(response), **kwargs) - assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) + assert status == CheckStatus({"hexanator/0"}, {"hexanator/0"}, ANY) @pytest.fixture diff --git a/tests/unit/test_idle_check_subordinate.py b/tests/unit/test_idle_check_subordinate.py index 149e468b..6d4fc237 100644 --- a/tests/unit/test_idle_check_subordinate.py +++ b/tests/unit/test_idle_check_subordinate.py @@ -16,7 +16,11 @@ def test_subordinate_apps(response: dict[str, Any], kwargs): status = check(convert(response), **kwargs) - assert status == CheckStatus({"ntp/0", "ubuntu/0"}, {"ntp/0", "ubuntu/0"}, set()) + assert status == CheckStatus( + {"ntp/0", "ubuntu/0"}, + {"ntp/0", "ubuntu/0"}, + {"ntp/0", "ubuntu/0"}, + ) def test_subordinate_is_selective(response, kwargs): @@ -25,7 +29,11 @@ def test_subordinate_is_selective(response, kwargs): ] subordinates["some-other/0"] = subordinates["ntp/0"] status = check(convert(response), **kwargs) - assert status == CheckStatus({"ntp/0", "ubuntu/0"}, {"ntp/0", "ubuntu/0"}, set()) + assert status == CheckStatus( + {"ntp/0", "ubuntu/0"}, + {"ntp/0", "ubuntu/0"}, + {"ntp/0", "ubuntu/0"}, + ) @pytest.fixture From 138d8f9058e1023e01a01d587b81949433da61ce Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 12:39:27 +0900 Subject: [PATCH 3/8] chore: better logging in integration tests --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index 2d676029..beae6956 100644 --- a/tox.ini +++ b/tox.ini @@ -60,7 +60,7 @@ commands = # it doesn't get run in CI envdir = {toxworkdir}/py3 commands = - pytest --tb native -s {posargs:-m 'serial'} + pytest --tb native {posargs:-m 'serial'} [testenv:validate] envdir = {toxworkdir}/validate From 1951425e935ab30fe2906d92cfc4ad523e572686 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 13:20:47 +0900 Subject: [PATCH 4/8] chore: studpi bug --- juju/model/__init__.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/juju/model/__init__.py b/juju/model/__init__.py index f40738b1..951713a7 100644 --- a/juju/model/__init__.py +++ b/juju/model/__init__.py @@ -3302,13 +3302,14 @@ async def new_wait_for_idle( deadline = None if timeout is None else started + timeout async def status_on_demand(): - yield _idle.check( - await self.get_status(), - apps=apps, - raise_on_error=raise_on_error, - raise_on_blocked=raise_on_blocked, - status=status, - ) + while True: + yield _idle.check( + await self.get_status(), + apps=apps, + raise_on_error=raise_on_error, + raise_on_blocked=raise_on_blocked, + status=status, + ) async for done in _idle.loop( status_on_demand(), From c6087a308c4fdda9f64355ea5d7ea863895dce7e Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 15:21:51 +0900 Subject: [PATCH 5/8] fix: async filter should output only one result for each input --- juju/model/_idle.py | 8 +++--- juju/version.py | 2 +- pyproject.toml | 2 +- tests/unit/test_idle_loop.py | 49 ++++++++++++++++++++++++++++++------ tox.ini | 1 + 5 files changed, 49 insertions(+), 13 deletions(-) diff --git a/juju/model/_idle.py b/juju/model/_idle.py index 6737fe12..7ac32c76 100644 --- a/juju/model/_idle.py +++ b/juju/model/_idle.py @@ -80,7 +80,7 @@ async def loop( wait_for_units, ) yield False - continue + break if ( wait_for_exact_units is not None @@ -93,9 +93,9 @@ async def loop( wait_for_exact_units, ) yield False - continue - - yield True + break + else: + yield True def check( diff --git a/juju/version.py b/juju/version.py index 86177ffd..c0cd876c 100644 --- a/juju/version.py +++ b/juju/version.py @@ -6,4 +6,4 @@ DEFAULT_ARCHITECTURE = "amd64" -CLIENT_VERSION = "3.6.1.0rc3" +CLIENT_VERSION = "3.6.1.0rc4" diff --git a/pyproject.toml b/pyproject.toml index d2eec1cc..6eedab93 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "juju" -version = "3.6.1.0rc3" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION +version = "3.6.1.0rc4" # Stop-gap until dynamic versioning is done; must be in sync with juju/version.py:CLIENT_VERSION description = "Python library for Juju" readme = "docs/readme.rst" license = { file = "LICENSE" } diff --git a/tests/unit/test_idle_loop.py b/tests/unit/test_idle_loop.py index 34a8c672..491231df 100644 --- a/tests/unit/test_idle_loop.py +++ b/tests/unit/test_idle_loop.py @@ -2,12 +2,10 @@ # Licensed under the Apache V2, see LICENCE file for details. from __future__ import annotations -import pytest from freezegun import freeze_time from juju.model._idle import CheckStatus, loop - # Missing tests # # FIXME hexanator idle period 1 @@ -18,13 +16,15 @@ # FIXME expected idle 1s below # FIXME idle period 1 # FIXME sending status=None, meaning some apps are still missing -# -@pytest.mark.xfail(reason="FIXME I misunderstood what 'idle' means") + + async def test_at_least_units(): async def checks(): - yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0"}, set()) - yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1"}, set()) - yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1", "u/2"}, set()) + yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0"}, {"u/0", "u/1", "u/2"}) + yield CheckStatus({"u/0", "u/1", "u/2"}, {"u/0", "u/1"}, {"u/0", "u/1", "u/2"}) + yield CheckStatus( + {"u/0", "u/1", "u/2"}, {"u/0", "u/1", "u/2"}, {"u/0", "u/1", "u/2"} + ) with freeze_time(): assert [ @@ -38,6 +38,41 @@ async def checks(): ] == [False, True, True] +async def test_for_exact_units(): + good = CheckStatus( + {"u/0", "u/1", "u/2"}, + {"u/1", "u/2"}, + {"u/0", "u/1", "u/2"}, + ) + too_few = CheckStatus( + {"u/0", "u/1", "u/2"}, + {"u/2"}, + {"u/0", "u/1", "u/2"}, + ) + too_many = CheckStatus( + {"u/0", "u/1", "u/2"}, + {"u/1", "u/2", "u/0"}, + {"u/0", "u/1", "u/2"}, + ) + + async def checks(): + yield too_few + yield good + yield too_many + yield good + + assert [ + v + async for v in loop( + checks(), + apps=frozenset(["u"]), + wait_for_units=1, + wait_for_exact_units=2, + idle_period=0, + ) + ] == [False, True, False, True] + + async def test_ping_pong(): good = CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) bad = CheckStatus({"hexanator/0"}, set(), set()) diff --git a/tox.ini b/tox.ini index beae6956..5356d45f 100644 --- a/tox.ini +++ b/tox.ini @@ -43,6 +43,7 @@ commands = envdir = {toxworkdir}/py3 commands = pytest \ + --log-level=DEBUG \ --tb native \ -m 'not serial' \ {posargs} \ From 3e29b2b8b0aa148dd4e32ca5e157ea46d855da0d Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 16:13:01 +0900 Subject: [PATCH 6/8] chore: add missing tests and refactor for readability --- tests/unit/test_idle_loop.py | 76 +++++++++++++++++++++++------------- 1 file changed, 48 insertions(+), 28 deletions(-) diff --git a/tests/unit/test_idle_loop.py b/tests/unit/test_idle_loop.py index 491231df..eaee5aa0 100644 --- a/tests/unit/test_idle_loop.py +++ b/tests/unit/test_idle_loop.py @@ -6,16 +6,24 @@ from juju.model._idle import CheckStatus, loop -# Missing tests -# -# FIXME hexanator idle period 1 -# FIXME workload maintenance, idle period 0 -# FIXME test exact count == 2 -# FIXME test exact count != 2 (1, 3) -# FIXME exact count vs wait_for_units -# FIXME expected idle 1s below -# FIXME idle period 1 -# FIXME sending status=None, meaning some apps are still missing + +async def alist(agen): + return [v async for v in agen] + + +async def test_wait_for_apps(): + async def checks(): + yield None + yield None + + assert await alist( + loop( + checks(), + apps=frozenset(["a"]), + wait_for_units=0, + idle_period=0, + ) + ) == [False, False] async def test_at_least_units(): @@ -27,15 +35,14 @@ async def checks(): ) with freeze_time(): - assert [ - v - async for v in loop( + assert await alist( + loop( checks(), apps=frozenset(["u"]), wait_for_units=2, idle_period=0, ) - ] == [False, True, True] + ) == [False, True, True] async def test_for_exact_units(): @@ -61,36 +68,49 @@ async def checks(): yield too_many yield good - assert [ - v - async for v in loop( + assert await alist( + loop( checks(), apps=frozenset(["u"]), wait_for_units=1, wait_for_exact_units=2, idle_period=0, ) - ] == [False, True, False, True] + ) == [False, True, False, True] -async def test_ping_pong(): - good = CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) - bad = CheckStatus({"hexanator/0"}, set(), set()) +async def test_idle_ping_pong(): + good = CheckStatus({"hexanator/0"}, {"hexanator/0"}, {"hexanator/0"}) + bad = CheckStatus({"hexanator/0"}, {"hexanator/0"}, set()) async def checks(): with freeze_time() as clock: - for _ in range(3): - yield good + for status in [good, bad, good, bad]: + yield status clock.tick(10) - yield bad + + assert await alist( + loop( + checks(), + apps=frozenset(["hexanator"]), + wait_for_units=1, + idle_period=15, + ) + ) == [False, False, False, False] + + +async def test_idle_period(): + async def checks(): + with freeze_time() as clock: + for _ in range(4): + yield CheckStatus({"hexanator/0"}, {"hexanator/0"}, {"hexanator/0"}) clock.tick(10) - assert [ - v - async for v in loop( + assert await alist( + loop( checks(), apps=frozenset(["hexanator"]), wait_for_units=1, idle_period=15, ) - ] == [False] * 6 + ) == [False, False, True, True] From 4276d99ca99b22cc552a1840e196da6b9088e35d Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 17:01:27 +0900 Subject: [PATCH 7/8] chore: slightly better type hints --- juju/model/_idle.py | 6 +++--- tests/unit/test_idle_loop.py | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/juju/model/_idle.py b/juju/model/_idle.py index 7ac32c76..7ca0cac9 100644 --- a/juju/model/_idle.py +++ b/juju/model/_idle.py @@ -7,7 +7,7 @@ import logging import time from dataclasses import dataclass -from typing import AsyncIterable +from typing import AbstractSet, AsyncIterable from ..client._definitions import ( ApplicationStatus, @@ -35,7 +35,7 @@ class CheckStatus: async def loop( foo: AsyncIterable[CheckStatus | None], *, - apps: frozenset[str], + apps: AbstractSet[str], wait_for_exact_units: int | None = None, wait_for_units: int, idle_period: float, @@ -101,7 +101,7 @@ async def loop( def check( full_status: FullStatus, *, - apps: frozenset[str], + apps: AbstractSet[str], raise_on_error: bool, raise_on_blocked: bool, status: str | None, diff --git a/tests/unit/test_idle_loop.py b/tests/unit/test_idle_loop.py index eaee5aa0..2e97bd67 100644 --- a/tests/unit/test_idle_loop.py +++ b/tests/unit/test_idle_loop.py @@ -19,7 +19,7 @@ async def checks(): assert await alist( loop( checks(), - apps=frozenset(["a"]), + apps={"a"}, wait_for_units=0, idle_period=0, ) @@ -38,7 +38,7 @@ async def checks(): assert await alist( loop( checks(), - apps=frozenset(["u"]), + apps={"u"}, wait_for_units=2, idle_period=0, ) @@ -71,7 +71,7 @@ async def checks(): assert await alist( loop( checks(), - apps=frozenset(["u"]), + apps={"u"}, wait_for_units=1, wait_for_exact_units=2, idle_period=0, @@ -92,7 +92,7 @@ async def checks(): assert await alist( loop( checks(), - apps=frozenset(["hexanator"]), + apps={"hexanator"}, wait_for_units=1, idle_period=15, ) @@ -109,7 +109,7 @@ async def checks(): assert await alist( loop( checks(), - apps=frozenset(["hexanator"]), + apps={"hexanator"}, wait_for_units=1, idle_period=15, ) From d7abe10ed5cb7e705c432a956d10a0499ac499e2 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 18 Dec 2024 17:20:16 +0900 Subject: [PATCH 8/8] chore: note on converted responses --- juju/client/facade.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/juju/client/facade.py b/juju/client/facade.py index 7ab4fddd..6fd34934 100644 --- a/juju/client/facade.py +++ b/juju/client/facade.py @@ -499,10 +499,14 @@ def _convert_response(response: dict[str, Any], *, cls: type[Type] | None) -> An if cls is None: return response if "error" in response: + # TODO: I don't think this ever happens, + # errors are handled by Connection.rpc(), + # though, admittedly the shape is different. cls = CLASSES["Error"] if typing_inspect.is_generic_type(cls) and issubclass( typing_inspect.get_origin(cls), Sequence ): + # TODO: I'm not sure this ever happens either. parameters = typing_inspect.get_parameters(cls) result = [] item_cls = parameters[0]