From a0fac06c41477cece5422fd86c9c56f2a616ec6c Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 20 Nov 2024 17:35:38 +0900 Subject: [PATCH 01/20] chore: pull types from wip PR for upcoming Model.wait_for_idle --- juju/model.py | 67 ++++++++++++++++++++++++++++++++++++++------------ juju/status.py | 21 ++++++++-------- juju/unit.py | 2 ++ juju/url.py | 18 ++++++++++---- pyproject.toml | 2 +- setup.py | 2 +- 6 files changed, 79 insertions(+), 33 deletions(-) diff --git a/juju/model.py b/juju/model.py index ada4ac29..e95e6248 100644 --- a/juju/model.py +++ b/juju/model.py @@ -19,7 +19,7 @@ from datetime import datetime, timedelta from functools import partial from pathlib import Path -from typing import Any +from typing import TYPE_CHECKING, Any, Literal, Mapping, overload import websockets import yaml @@ -58,6 +58,13 @@ from .url import URL, Schema from .version import DEFAULT_ARCHITECTURE +if TYPE_CHECKING: + from .application import Application + from .machine import Machine + from .relation import Relation + from .remoteapplication import ApplicationOffer, RemoteApplication + from .unit import Unit + log = logging.getLogger(__name__) @@ -134,7 +141,35 @@ def __init__(self, model): self.model = model self.state = dict() - def _live_entity_map(self, entity_type): + @overload + def _live_entity_map( + self, entity_type: Literal["application"] + ) -> dict[str, Application]: ... + + @overload + def _live_entity_map( + self, entity_type: Literal["applicationOffer"] + ) -> dict[str, ApplicationOffer]: ... + + @overload + def _live_entity_map( + self, entity_type: Literal["machine"] + ) -> dict[str, Machine]: ... + + @overload + def _live_entity_map( + self, entity_type: Literal["relation"] + ) -> dict[str, Relation]: ... + + @overload + def _live_entity_map( + self, entity_type: Literal["remoteApplication"] + ) -> dict[str, RemoteApplication]: ... + + @overload + def _live_entity_map(self, entity_type: Literal["unit"]) -> dict[str, Unit]: ... + + def _live_entity_map(self, entity_type: str) -> Mapping[str, ModelEntity]: """Return an id:Entity map of all the living entities of type ``entity_type``. @@ -146,7 +181,7 @@ def _live_entity_map(self, entity_type): } @property - def applications(self): + def applications(self) -> dict[str, Application]: """Return a map of application-name:Application for all applications currently in the model. @@ -154,7 +189,7 @@ def applications(self): return self._live_entity_map("application") @property - def remote_applications(self): + def remote_applications(self) -> dict[str, RemoteApplication]: """Return a map of application-name:Application for all remote applications currently in the model. @@ -162,14 +197,14 @@ def remote_applications(self): return self._live_entity_map("remoteApplication") @property - def application_offers(self): + def application_offers(self) -> dict[str, ApplicationOffer]: """Return a map of application-name:Application for all applications offers currently in the model. """ return self._live_entity_map("applicationOffer") @property - def machines(self): + def machines(self) -> dict[str, Machine]: """Return a map of machine-id:Machine for all machines currently in the model. @@ -177,7 +212,7 @@ def machines(self): return self._live_entity_map("machine") @property - def units(self): + def units(self) -> dict[str, Unit]: """Return a map of unit-id:Unit for all units currently in the model. @@ -185,12 +220,12 @@ def units(self): return self._live_entity_map("unit") @property - def subordinate_units(self): + def subordinate_units(self) -> dict[str, Unit]: """Return a map of unit-id:Unit for all subordinate units""" return {u_name: u for u_name, u in self.units.items() if u.is_subordinate} @property - def relations(self): + def relations(self) -> dict[str, Relation]: """Return a map of relation-id:Relation for all relations currently in the model. @@ -1110,7 +1145,7 @@ def tag(self): return tag.model(self.uuid) @property - def applications(self): + def applications(self) -> dict[str, Application]: """Return a map of application-name:Application for all applications currently in the model. @@ -1118,7 +1153,7 @@ def applications(self): return self.state.applications @property - def remote_applications(self): + def remote_applications(self) -> dict[str, RemoteApplication]: """Return a map of application-name:Application for all remote applications currently in the model. @@ -1126,14 +1161,14 @@ def remote_applications(self): return self.state.remote_applications @property - def application_offers(self): + def application_offers(self) -> dict[str, ApplicationOffer]: """Return a map of application-name:Application for all applications offers currently in the model. """ return self.state.application_offers @property - def machines(self): + def machines(self) -> dict[str, Machine]: """Return a map of machine-id:Machine for all machines currently in the model. @@ -1141,7 +1176,7 @@ def machines(self): return self.state.machines @property - def units(self): + def units(self) -> dict[str, Unit]: """Return a map of unit-id:Unit for all units currently in the model. @@ -1149,7 +1184,7 @@ def units(self): return self.state.units @property - def subordinate_units(self): + def subordinate_units(self) -> dict[str, Unit]: """Return a map of unit-id:Unit for all subordiante units currently in the model. @@ -1157,7 +1192,7 @@ def subordinate_units(self): return self.state.subordinate_units @property - def relations(self): + def relations(self) -> list[Relation]: """Return a list of all Relations currently in the model.""" return list(self.state.relations.values()) diff --git a/juju/status.py b/juju/status.py index 81ebbd9b..8825f61f 100644 --- a/juju/status.py +++ b/juju/status.py @@ -1,5 +1,6 @@ # Copyright 2023 Canonical Ltd. # Licensed under the Apache V2, see LICENCE file for details. +from __future__ import annotations import logging import warnings @@ -8,26 +9,26 @@ log = logging.getLogger(__name__) -""" derive_status is used to determine the application status from a set of unit -status values. -:param statues: list of known unit workload statues - -""" +def derive_status(statuses: list[str]): + """Derive status from a set. + derive_status is used to determine the application status from a set of unit + status values. -def derive_status(statues): + :param statuses: list of known unit workload statuses + """ current = "unknown" - for status in statues: - if status in severities and severities[status] > severities[current]: + for status in statuses: + if status in severity_map and severity_map[status] > severity_map[current]: current = status return current -""" severities holds status values with a severity measure. +""" severity_map holds status values with a severity measure. Status values with higher severity are used in preference to others. """ -severities = { +severity_map = { "error": 100, "blocked": 90, "waiting": 80, diff --git a/juju/unit.py b/juju/unit.py index b0d66a49..4cb8e259 100644 --- a/juju/unit.py +++ b/juju/unit.py @@ -15,6 +15,8 @@ class Unit(model.ModelEntity): + name: str + @property def agent_status(self): """Returns the current agent status string.""" diff --git a/juju/url.py b/juju/url.py index 1f915bff..27772e46 100644 --- a/juju/url.py +++ b/juju/url.py @@ -1,5 +1,6 @@ # Copyright 2023 Canonical Ltd. # Licensed under the Apache V2, see LICENCE file for details. +from __future__ import annotations from enum import Enum from urllib.parse import urlparse @@ -8,6 +9,8 @@ class Schema(Enum): + """Charm URL schema kinds.""" + LOCAL = "local" CHARM_STORE = "cs" CHARM_HUB = "ch" @@ -20,18 +23,23 @@ def __str__(self): class URL: + """Private URL class for this library internals only.""" + + name: str + def __init__( self, schema, user=None, - name=None, + name: str | None = None, revision=None, series=None, architecture=None, ): self.schema = schema self.user = user - self.name = name + # the parse method will set the correct value later + self.name = name # type: ignore self.series = series # 0 can be a valid revision, hence the more verbose check. @@ -41,7 +49,7 @@ def __init__( self.architecture = architecture @staticmethod - def parse(s, default_store=Schema.CHARM_HUB): + def parse(s: str, default_store=Schema.CHARM_HUB) -> URL: """Parse parses the provided charm URL string into its respective structure. @@ -103,7 +111,7 @@ def __str__(self): return f"{self.schema!s}:{self.path()}" -def parse_v1_url(schema, u, s): +def parse_v1_url(schema, u, s) -> URL: c = URL(schema) parts = u.path.split("/") @@ -135,7 +143,7 @@ def parse_v1_url(schema, u, s): return c -def parse_v2_url(u, s, default_store): +def parse_v2_url(u, s, default_store) -> URL: if not u.scheme: c = URL(default_store) elif Schema.CHARM_HUB.matches(u.scheme): diff --git a/pyproject.toml b/pyproject.toml index 722cbf92..6974f630 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -25,7 +25,7 @@ dependencies = [ "macaroonbakery>=1.1,<2.0", "pyRFC3339>=1.0,<2.0", "pyyaml>=5.1.2", - "websockets>=8.1,<14.0", + "websockets>=13.0.1,<14.0", "paramiko>=2.4.0", "pyasn1>=0.4.4", "toposort>=1.5,<2", diff --git a/setup.py b/setup.py index 0c22a6e0..d3464ec6 100644 --- a/setup.py +++ b/setup.py @@ -23,7 +23,7 @@ "macaroonbakery>=1.1,<2.0", "pyRFC3339>=1.0,<2.0", "pyyaml>=5.1.2", - "websockets>=8.1,<14.0", + "websockets>=13.0.1,<14.0", "paramiko>=2.4.0", "pyasn1>=0.4.4", "toposort>=1.5,<2", From ee6414e4ad6fb2d7b3bdb9d63c90a44d49c3da2d Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Wed, 20 Nov 2024 17:51:46 +0900 Subject: [PATCH 02/20] chore: type hints for wait_for_idle specifically --- juju/model.py | 60 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/juju/model.py b/juju/model.py index e95e6248..81648a89 100644 --- a/juju/model.py +++ b/juju/model.py @@ -28,8 +28,7 @@ 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, connector -from .client.connection import Connection +from .client import client, connection, connector from .client.overrides import Caveat, Macaroon from .constraints import parse as parse_constraints from .controller import ConnectedController, Controller @@ -60,6 +59,7 @@ if TYPE_CHECKING: from .application import Application + from .client._definitions import FullStatus from .machine import Machine from .relation import Relation from .remoteapplication import ApplicationOffer, RemoteApplication @@ -267,7 +267,9 @@ def apply_delta(self, delta): entity = self.get_entity(delta.entity, delta.get_id()) return entity.previous(), entity - def get_entity(self, entity_type, entity_id, history_index=-1, connected=True): + def get_entity( + self, entity_type, entity_id, history_index=-1, connected=True + ) -> ModelEntity | None: """Return an object instance for the given entity_type and id. By default the object state matches the most recent state from @@ -295,6 +297,11 @@ class ModelEntity: """An object in the Model tree""" entity_id: str + model: Model + _history_index: int + connected: bool + connection: connection.Connection + _status: str def __init__( self, @@ -616,6 +623,9 @@ async def resolve( class Model: """The main API for interacting with a Juju model.""" + connector: connector.Connector + state: ModelState + def __init__( self, max_frame_size=None, @@ -660,7 +670,7 @@ def is_connected(self): """Reports whether the Model is currently connected.""" return self._connector.is_connected() - def connection(self) -> Connection: + def connection(self) -> connection.Connection: """Return the current Connection object. It raises an exception if the Model is disconnected """ @@ -914,7 +924,10 @@ def add_local_charm(self, charm_file, series="", size=None): instead. """ - conn, headers, path_prefix = self.connection().https_connection() + connection = self.connection() + assert connection + + conn, headers, path_prefix = connection.https_connection() path = "%s/charms?series=%s" % (path_prefix, series) headers["Content-Type"] = "application/zip" if size: @@ -1212,11 +1225,12 @@ def name(self): return self._info.name @property - def info(self): + def info(self) -> ModelInfo: """Return the cached client.ModelInfo object for this Model. If Model.get_info() has not been called, this will return None. """ + assert self._info is not None return self._info @property @@ -1306,11 +1320,13 @@ async def _all_watcher(): del allwatcher.Id continue except websockets.ConnectionClosed: - monitor = self.connection().monitor + connection = self.connection() + assert connection + monitor = connection.monitor if monitor.status == monitor.ERROR: # closed unexpectedly, try to reopen log.warning("Watcher: connection closed, reopening") - await self.connection().reconnect() + await connection.reconnect() if monitor.status != monitor.CONNECTED: # reconnect failed; abort and shutdown log.error( @@ -2624,7 +2640,7 @@ 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): + async def get_status(self, filters=None, utc=False) -> FullStatus: """Return the status of the model. :param str filters: Optional list of applications, units, or machines @@ -2959,15 +2975,15 @@ async def _get_source_api(self, url): async def wait_for_idle( self, apps: list[str] | None = None, - raise_on_error=True, - raise_on_blocked=False, - wait_for_active=False, - timeout=10 * 60, - idle_period=15, - check_freq=0.5, - status=None, - wait_for_at_least_units=None, - wait_for_exact_units=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. @@ -3035,12 +3051,12 @@ async def wait_for_idle( raise JujuError(f"Expected a List[str] for apps, given {apps}") apps = apps or self.applications - idle_times = {} - units_ready = set() # The units that are in the desired state - last_log_time = None + idle_times: dict[str, datetime] = {} + units_ready: set[str] = set() # The units that are in the desired state + last_log_time: datetime | None = None log_interval = timedelta(seconds=30) - def _raise_for_status(entities, status): + def _raise_for_status(entities: dict[str, list[str]], status: Any): if not entities: return for entity_name, error_type in ( From 99d8fc11fcc7687a91dcadb5d527df33278f22cb Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 12:23:39 +0900 Subject: [PATCH 03/20] chore: no need to `assert connection` --- juju/client/connection.py | 24 +++++++++--------------- juju/client/connector.py | 3 ++- juju/model.py | 17 +++++++---------- 3 files changed, 18 insertions(+), 26 deletions(-) diff --git a/juju/client/connection.py b/juju/client/connection.py index e79f2ea7..88c31c2a 100644 --- a/juju/client/connection.py +++ b/juju/client/connection.py @@ -27,7 +27,7 @@ from .facade_versions import client_facade_versions, known_unsupported_facades SpecifiedFacades: TypeAlias = "dict[str, dict[Literal['versions'], Sequence[int]]]" -_WebSocket: TypeAlias = "websockets.legacy.client.WebSocketClientProtocol" +_WebSocket: TypeAlias = websockets.WebSocketClientProtocol LEVELS = ["TRACE", "DEBUG", "INFO", "WARNING", "ERROR"] log = logging.getLogger("juju.client.connection") @@ -291,7 +291,7 @@ def is_using_old_client(self): def is_open(self): return self.monitor.status == Monitor.CONNECTED - def _get_ssl(self, cert=None): + def _get_ssl(self, cert: str | None = None) -> ssl.SSLContext: context = ssl.create_default_context( purpose=ssl.Purpose.SERVER_AUTH, cadata=cert ) @@ -305,7 +305,9 @@ def _get_ssl(self, cert=None): context.check_hostname = False return context - async def _open(self, endpoint, cacert) -> tuple[_WebSocket, str, str, str]: + async def _open( + self, endpoint: str, cacert: str + ) -> tuple[_WebSocket, str, str, str]: if self.is_debug_log_connection: assert self.uuid url = f"wss://user-{self.username}:{self.password}@{endpoint}/model/{self.uuid}/log" @@ -323,10 +325,6 @@ async def _open(self, endpoint, cacert) -> tuple[_WebSocket, str, str, str]: sock = self.proxy.socket() server_hostname = "juju-app" - def _exit_tasks(): - for task in jasyncio.all_tasks(): - task.cancel() - return ( ( await websockets.connect( @@ -342,7 +340,7 @@ def _exit_tasks(): cacert, ) - async def close(self, to_reconnect=False): + async def close(self, to_reconnect: bool = False): if not self._ws: return self.monitor.close_called.set() @@ -380,11 +378,7 @@ async def close(self, to_reconnect=False): async def _recv(self, request_id: int) -> dict[str, Any]: if not self.is_open: - raise websockets.exceptions.ConnectionClosed( - websockets.frames.Close( - websockets.frames.CloseCode.NORMAL_CLOSURE, "websocket closed" - ) - ) + raise websockets.exceptions.ConnectionClosedOK(None, None) try: return await self.messages.get(request_id) except GeneratorExit: @@ -626,7 +620,7 @@ async def rpc( return result - def _http_headers(self): + def _http_headers(self) -> dict[str, str]: """Return dictionary of http headers necessary for making an http connection to the endpoint of this Connection. @@ -640,7 +634,7 @@ def _http_headers(self): token = base64.b64encode(creds.encode()) return {"Authorization": f"Basic {token.decode()}"} - def https_connection(self): + def https_connection(self) -> tuple[HTTPSConnection, dict[str, str], str]: """Return an https connection to this Connection's endpoint. Returns a 3-tuple containing:: diff --git a/juju/client/connector.py b/juju/client/connector.py index c9be0cda..cc307902 100644 --- a/juju/client/connector.py +++ b/juju/client/connector.py @@ -50,7 +50,7 @@ def __init__( self.model_name = None self.jujudata = jujudata or FileJujuData() - def is_connected(self): + def is_connected(self) -> bool: """Report whether there is a currently connected controller or not""" return self._connection is not None @@ -60,6 +60,7 @@ def connection(self) -> Connection: """ if not self.is_connected(): raise NoConnectionException("not connected") + assert self._connection return self._connection async def connect(self, **kwargs): diff --git a/juju/model.py b/juju/model.py index 81648a89..ccd5cfbe 100644 --- a/juju/model.py +++ b/juju/model.py @@ -924,10 +924,7 @@ def add_local_charm(self, charm_file, series="", size=None): instead. """ - connection = self.connection() - assert connection - - conn, headers, path_prefix = connection.https_connection() + conn, headers, path_prefix = self.connection().https_connection() path = "%s/charms?series=%s" % (path_prefix, series) headers["Content-Type"] = "application/zip" if size: @@ -1320,14 +1317,14 @@ async def _all_watcher(): del allwatcher.Id continue except websockets.ConnectionClosed: - connection = self.connection() - assert connection - monitor = connection.monitor - if monitor.status == monitor.ERROR: + if self.connection().monitor.status == connection.Monitor.ERROR: # closed unexpectedly, try to reopen log.warning("Watcher: connection closed, reopening") - await connection.reconnect() - if monitor.status != monitor.CONNECTED: + await self.connection().reconnect() + if ( + self.connection().monitor.status + != connection.Monitor.CONNECTED + ): # reconnect failed; abort and shutdown log.error( "Watcher: automatic reconnect " From 8cf92b32d5cbe0faa8bab57a42ad6c811f9672e9 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 13:00:57 +0900 Subject: [PATCH 04/20] chore: use StatusStr for known status values --- juju/status.py | 62 ++++++++++++++++++++++++++++++++++++-------------- pyproject.toml | 7 +++--- setup.py | 1 + 3 files changed, 50 insertions(+), 20 deletions(-) diff --git a/juju/status.py b/juju/status.py index 8825f61f..624cda21 100644 --- a/juju/status.py +++ b/juju/status.py @@ -3,14 +3,50 @@ from __future__ import annotations import logging +import sys import warnings +if sys.version_info >= (3, 11): + from enum import StrEnum +else: + from backports.strenum import StrEnum + from .client import client log = logging.getLogger(__name__) -def derive_status(statuses: list[str]): +class StatusStr(StrEnum): + """Recognised status values. + + Please keep this set exact same as the severity map below. + """ + + error = "error" + blocked = "blocked" + waiting = "waiting" + maintenance = "maintenance" + active = "active" + terminated = "terminated" + unknown = "unknown" + + +""" severity_map holds status values with a severity measure. +Status values with higher severity are used in preference to others. +""" +severity_map: dict[StatusStr, int] = { + # FIXME: Juju defines a lot more status values #1204 + StatusStr.error: 100, + StatusStr.blocked: 90, + StatusStr.waiting: 80, + StatusStr.maintenance: 70, + StatusStr.active: 60, + StatusStr.terminated: 50, + StatusStr.unknown: 40, +} + + +def derive_status(statuses: list[str | StatusStr]) -> StatusStr: """Derive status from a set. derive_status is used to determine the application status from a set of unit @@ -18,27 +54,19 @@ def derive_status(statuses: list[str]): :param statuses: list of known unit workload statuses """ - current = "unknown" + current: StatusStr = StatusStr.unknown for status in statuses: - if status in severity_map and severity_map[status] > severity_map[current]: + try: + status = StatusStr(status) + except ValueError: + continue + if (new_level := severity_map.get(status)) and new_level > severity_map[ + current + ]: current = status return current -""" severity_map holds status values with a severity measure. -Status values with higher severity are used in preference to others. -""" -severity_map = { - "error": 100, - "blocked": 90, - "waiting": 80, - "maintenance": 70, - "active": 60, - "terminated": 50, - "unknown": 40, -} - - async def formatted_status(model, target=None, raw=False, filters=None): """Returns a string that mimics the content of the information returned in the juju status command. If the raw parameter is diff --git a/pyproject.toml b/pyproject.toml index 6974f630..47c22809 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,7 +5,7 @@ description = "Python library for Juju" readme = "docs/readme.rst" license = { file = "LICENSE" } maintainers = [{name = "Juju Ecosystem Engineering", email = "juju@lists.ubuntu.com"}] -requires-python = ">=3.8" +requires-python = ">=3.8.6" classifiers = [ "Development Status :: 5 - Production/Stable", "Intended Audience :: Developers", @@ -33,7 +33,8 @@ dependencies = [ "kubernetes>=12.0.1,<31.0.0", "hvac", "packaging", - "typing-extensions>=4.5.0" + "typing-extensions>=4.5.0", + "backports.strenum>=1.3.1", ] [project.urls] @@ -212,7 +213,7 @@ ignore = [ [tool.pyright] # These are tentative include = ["**/*.py"] -pythonVersion = "3.8" +pythonVersion = "3.8.6" typeCheckingMode = "strict" useLibraryCodeForTypes = true reportGeneralTypeIssues = true diff --git a/setup.py b/setup.py index d3464ec6..5b845661 100644 --- a/setup.py +++ b/setup.py @@ -32,6 +32,7 @@ "hvac", "packaging", "typing-extensions>=4.5.0", + "backports.strenum", ], include_package_data=True, maintainer="Juju Ecosystem Engineering", From bc93dfda3e642dd70535e5d4078ae370ee38f7e0 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 13:13:28 +0900 Subject: [PATCH 05/20] chore: update tox.ini for the new pacakge --- tox.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tox.ini b/tox.ini index 0e97e6c8..43abbec2 100644 --- a/tox.ini +++ b/tox.ini @@ -18,6 +18,7 @@ passenv = HOME TEST_AGENTS LXD_DIR +# FIXME would it be easier to `pip install -e .`? deps = macaroonbakery toposort @@ -32,6 +33,7 @@ deps = hvac packaging setuptools + backports.strenum [testenv:docs] deps = From 945bac4767f198ec78c04c7549a9529044b2bfb8 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 13:14:31 +0900 Subject: [PATCH 06/20] chore: keep python versions independent in testing --- .github/workflows/test.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index c9827512..1aef854a 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -46,6 +46,7 @@ jobs: name: Unit tests runs-on: ubuntu-latest strategy: + fail-fast: false matrix: python: - "3.8" From af5fb474554ae5039e0c72d4a5141f2982edf26d Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 15:12:04 +0900 Subject: [PATCH 07/20] chore: better url.URL docstring --- juju/url.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/juju/url.py b/juju/url.py index 27772e46..3ad90546 100644 --- a/juju/url.py +++ b/juju/url.py @@ -23,7 +23,10 @@ def __str__(self): class URL: - """Private URL class for this library internals only.""" + """Private URL class for this library internals only. + + Should be instantiated by `URL.parse` constructor. + """ name: str From bc881e47d16b89d2a28e109b411eb8baf25d92ea Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 15:20:16 +0900 Subject: [PATCH 08/20] chore: guard access to model.info on connection --- juju/model.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/juju/model.py b/juju/model.py index ccd5cfbe..c1773d6f 100644 --- a/juju/model.py +++ b/juju/model.py @@ -23,6 +23,7 @@ import websockets import yaml +from typing_extensions import deprecated from . import jasyncio, provisioner, tag, utils from .annotationhelper import _get_annotations, _set_annotations @@ -801,13 +802,14 @@ async def connect_current(self): """ return await self.connect() + @deprecated("Model.connect_to() is deprecated and will be removed soon") async def connect_to(self, connection): conn_params = connection.connect_params() await self._connect_direct(**conn_params) async def _connect_direct(self, **kwargs): - if self.info: - uuid = self.info.uuid + if self._info: + uuid = self._info.uuid elif "uuid" in kwargs: uuid = kwargs["uuid"] else: @@ -1227,6 +1229,9 @@ def info(self) -> ModelInfo: If Model.get_info() has not been called, this will return None. """ + if not self.is_connected(): + raise JujuModelError("Model is not connected") + assert self._info is not None return self._info From 35499f415db3e3f8d444b8574cafb4aaca763310 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 15:50:52 +0900 Subject: [PATCH 09/20] chore: simpler tox.ini --- pyproject.toml | 11 ++++++++++- setup.py | 12 +++++++++++- tox.ini | 34 +++++----------------------------- 3 files changed, 26 insertions(+), 31 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 47c22809..3ce3cd69 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,8 +34,17 @@ dependencies = [ "hvac", "packaging", "typing-extensions>=4.5.0", - "backports.strenum>=1.3.1", + 'backports.strenum>=1.3.1; python_version < "3.11"', ] +[project.optional-dependencies] +dev = [ + "typing-inspect", + "pytest", + "pytest-asyncio", + "Twine", + "setuptools", # ? + "pylxd", + ] [project.urls] "Homepage" = "https://juju.is/docs/sdk" diff --git a/setup.py b/setup.py index 5b845661..655a450b 100644 --- a/setup.py +++ b/setup.py @@ -32,8 +32,18 @@ "hvac", "packaging", "typing-extensions>=4.5.0", - "backports.strenum", + 'backports.strenum>=1.3.1; python_version < "3.11"', ], + extras_require={ + "dev": [ + "typing-inspect", + "pytest", + "pytest-asyncio", + "Twine", + "setuptools", # ? + "pylxd", + ] + }, include_package_data=True, maintainer="Juju Ecosystem Engineering", maintainer_email="juju@lists.ubuntu.com", diff --git a/tox.ini b/tox.ini index 43abbec2..89c8bcc3 100644 --- a/tox.ini +++ b/tox.ini @@ -9,31 +9,15 @@ envlist = py3,py38,py39,py310,py311,docs skipsdist=True [testenv] -usedevelop=True -commands = - pip install urllib3<2 - pip install pylxd - pytest --tb native -s -k 'not integration' -m 'not serial' {posargs} +use_develop = True +# This should work, but doesn't. Hence the deps= below +# extras = dev +deps = + .[dev] passenv = HOME TEST_AGENTS LXD_DIR -# FIXME would it be easier to `pip install -e .`? -deps = - macaroonbakery - toposort - typing-inspect - paramiko - ipdb - pytest - pytest-asyncio - Twine - websockets<14.0 - kubernetes<31.0.0 - hvac - packaging - setuptools - backports.strenum [testenv:docs] deps = @@ -47,8 +31,6 @@ commands = [testenv:integration] envdir = {toxworkdir}/py3 commands = - pip install urllib3<2 - pip install pylxd pytest \ --tb native \ -k 'integration' \ @@ -60,8 +42,6 @@ commands = [testenv:integration-quarantine] envdir = {toxworkdir}/py3 commands = - pip install urllib3<2 - pip install pylxd pytest \ --tb native \ -m 'not serial' \ @@ -72,8 +52,6 @@ commands = [testenv:unit] envdir = {toxworkdir}/py3 commands = - pip install urllib3<2 - pip install pylxd pytest {toxinidir}/tests/unit {posargs} [testenv:serial] @@ -82,8 +60,6 @@ commands = # it doesn't get run in CI envdir = {toxworkdir}/py3 commands = - pip install urllib3<2 - pip install pylxd pytest --tb native -s {posargs:-m 'serial'} [testenv:validate] From 018aa2da716801c1653f036294da8b1643fceac1 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 15:59:04 +0900 Subject: [PATCH 10/20] chore: test the new StatusStr vs severity_map --- tests/validate/test_status_severity.py | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 tests/validate/test_status_severity.py diff --git a/tests/validate/test_status_severity.py b/tests/validate/test_status_severity.py new file mode 100644 index 00000000..beec2588 --- /dev/null +++ b/tests/validate/test_status_severity.py @@ -0,0 +1,10 @@ +from juju.status import StatusStr, severity_map + + +def test_status_str_values(): + for name, value in StatusStr._member_map_.items(): + assert name == value + + +def test_severity_map(): + assert set(StatusStr._member_names_) == set(severity_map) From 3a56b5dba0fc97c16ce8ce72502a6d15896c9cce Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 16:01:01 +0900 Subject: [PATCH 11/20] chore: simplify worst status selection --- juju/status.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/juju/status.py b/juju/status.py index 624cda21..12336d47 100644 --- a/juju/status.py +++ b/juju/status.py @@ -59,10 +59,10 @@ def derive_status(statuses: list[str | StatusStr]) -> StatusStr: try: status = StatusStr(status) except ValueError: + # Unknown Juju status, let's assume it's least important continue - if (new_level := severity_map.get(status)) and new_level > severity_map[ - current - ]: + + if severity_map[status] > severity_map[current]: current = status return current From 3784fe8a432dfbe5e96e98dc850eea80e041ccaa Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 16:07:40 +0900 Subject: [PATCH 12/20] chore: try building in a newer environment --- .readthedocs.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.readthedocs.yaml b/.readthedocs.yaml index f861b63a..e4ee7e57 100644 --- a/.readthedocs.yaml +++ b/.readthedocs.yaml @@ -5,9 +5,9 @@ python: - requirements: docs/requirements.txt build: - os: ubuntu-22.04 + os: ubuntu-24.04 tools: - python: "3.10" + python: "3.13" sphinx: configuration: docs/conf.py From 2f078064f1304facc7385639245a90b3d1924f49 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 16:12:06 +0900 Subject: [PATCH 13/20] chore: rely on pyproject.toml when building docs --- docs/requirements.txt | 16 ---------------- tox.ini | 5 ++++- 2 files changed, 4 insertions(+), 17 deletions(-) delete mode 100644 docs/requirements.txt diff --git a/docs/requirements.txt b/docs/requirements.txt deleted file mode 100644 index 2ee6d200..00000000 --- a/docs/requirements.txt +++ /dev/null @@ -1,16 +0,0 @@ -pytz -pymacaroons -sphinx==5.3.0 -sphinxcontrib-asyncio -sphinx_rtd_theme -websockets -typing-inspect -pyyaml -pyasn1 -pyrfc3339 -paramiko -macaroonbakery -toposort -python-dateutil -kubernetes -packaging diff --git a/tox.ini b/tox.ini index 89c8bcc3..f3e80d42 100644 --- a/tox.ini +++ b/tox.ini @@ -21,7 +21,10 @@ passenv = [testenv:docs] deps = - -r docs/requirements.txt + .[dev] + sphinx==5.3.0 + sphinxcontrib-asyncio + sphinx_rtd_theme allowlist_externals = rm commands = From fbe1e63a6f75669b4ae5712969215cf0daee9b09 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 17:09:54 +0900 Subject: [PATCH 14/20] chore: make pylxd optional, as it brings in ws4py from 2018 --- pyproject.toml | 1 - setup.py | 1 - tests/integration/test_model.py | 14 +++++++++----- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 3ce3cd69..82fcfbe9 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,7 +43,6 @@ dev = [ "pytest-asyncio", "Twine", "setuptools", # ? - "pylxd", ] [project.urls] diff --git a/setup.py b/setup.py index 655a450b..d7af9560 100644 --- a/setup.py +++ b/setup.py @@ -41,7 +41,6 @@ "pytest-asyncio", "Twine", "setuptools", # ? - "pylxd", ] }, include_package_data=True, diff --git a/tests/integration/test_model.py b/tests/integration/test_model.py index e843a774..e4659480 100644 --- a/tests/integration/test_model.py +++ b/tests/integration/test_model.py @@ -10,7 +10,6 @@ from unittest import mock import paramiko -import pylxd import pytest from juju import jasyncio, tag, url @@ -29,6 +28,11 @@ from ..utils import GB, INTEGRATION_TEST_DIR, MB, OVERLAYS_DIR, SSH_KEY, TESTS_DIR +@pytest.fixture +def pylxd(): + return pytest.importorskip("pylxd") + + @base.bootstrapped async def test_model_name(): model = Model() @@ -532,7 +536,7 @@ async def test_add_machine(): assert len(model.machines) == 0 -async def add_manual_machine_ssh(is_root=False): +async def add_manual_machine_ssh(pylxd, is_root=False): # Verify controller is localhost async with base.CleanController() as controller: cloud = await controller.get_cloud() @@ -677,7 +681,7 @@ def wait_for_network(container, timeout=30): @base.bootstrapped -async def test_add_manual_machine_ssh(): +async def test_add_manual_machine_ssh(pylxd): """Test manual machine provisioning with a non-root user. Tests manual machine provisioning using a randomized username with @@ -687,9 +691,9 @@ async def test_add_manual_machine_ssh(): @base.bootstrapped -async def test_add_manual_machine_ssh_root(): +async def test_add_manual_machine_ssh_root(pylxd): """Test manual machine provisioning with the root user.""" - await add_manual_machine_ssh(is_root=True) + await add_manual_machine_ssh(pylxd, is_root=True) @base.bootstrapped From e2740a10f1bad0dd96b99ed354f6a2beb8d0eb1d Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 17:12:06 +0900 Subject: [PATCH 15/20] chore: update Model.info docstring --- juju/model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/juju/model.py b/juju/model.py index c1773d6f..8dd7d747 100644 --- a/juju/model.py +++ b/juju/model.py @@ -1227,7 +1227,7 @@ def name(self): def info(self) -> ModelInfo: """Return the cached client.ModelInfo object for this Model. - If Model.get_info() has not been called, this will return None. + If Model.get_info() has not been called, this will raise an error. """ if not self.is_connected(): raise JujuModelError("Model is not connected") From 2fdcaef9096e6aa7f53199974454660d2f0d3fc7 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 17:14:42 +0900 Subject: [PATCH 16/20] chore: dedent, drop setuptools in dev --- pyproject.toml | 11 +++++------ setup.py | 1 - 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 82fcfbe9..a2721bf5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,12 +38,11 @@ dependencies = [ ] [project.optional-dependencies] dev = [ - "typing-inspect", - "pytest", - "pytest-asyncio", - "Twine", - "setuptools", # ? - ] + "typing-inspect", + "pytest", + "pytest-asyncio", + "Twine", +] [project.urls] "Homepage" = "https://juju.is/docs/sdk" diff --git a/setup.py b/setup.py index d7af9560..5ee1fe8c 100644 --- a/setup.py +++ b/setup.py @@ -40,7 +40,6 @@ "pytest", "pytest-asyncio", "Twine", - "setuptools", # ? ] }, include_package_data=True, From 08b0e0b05aaa6fda02ae69edf0595e88acbb7b86 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 17:18:01 +0900 Subject: [PATCH 17/20] chore: readthedocs by trial and error --- .readthedocs.yaml | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.readthedocs.yaml b/.readthedocs.yaml index e4ee7e57..103a5ea2 100644 --- a/.readthedocs.yaml +++ b/.readthedocs.yaml @@ -2,7 +2,15 @@ version: 2 python: install: - - requirements: docs/requirements.txt + - method: pip + path: . + extra_requirements: + - dev + - method: pip + packages: + - sphinx==5.3.0 + - sphinxcontrib-asyncio + - sphinx_rtd_theme build: os: ubuntu-24.04 From 9f45c063a4d80a1fc862df9d81e9c8ae5c99a8dd Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 17:21:03 +0900 Subject: [PATCH 18/20] chore: readthedocs by trial and error --- .readthedocs.yaml | 6 +----- pyproject.toml | 5 +++++ tox.ini | 5 +---- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/.readthedocs.yaml b/.readthedocs.yaml index 103a5ea2..f51d87e3 100644 --- a/.readthedocs.yaml +++ b/.readthedocs.yaml @@ -6,11 +6,7 @@ python: path: . extra_requirements: - dev - - method: pip - packages: - - sphinx==5.3.0 - - sphinxcontrib-asyncio - - sphinx_rtd_theme + - docs build: os: ubuntu-24.04 diff --git a/pyproject.toml b/pyproject.toml index a2721bf5..f3c1118e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,6 +43,11 @@ dev = [ "pytest-asyncio", "Twine", ] +docs = [ + "sphinx==5.3.0", + "sphinxcontrib-asyncio", + "sphinx_rtd_theme", +] [project.urls] "Homepage" = "https://juju.is/docs/sdk" diff --git a/tox.ini b/tox.ini index f3e80d42..1e6071c0 100644 --- a/tox.ini +++ b/tox.ini @@ -21,10 +21,7 @@ passenv = [testenv:docs] deps = - .[dev] - sphinx==5.3.0 - sphinxcontrib-asyncio - sphinx_rtd_theme + .[dev,docs] allowlist_externals = rm commands = From f2c6c5098f0a056d30d9b1922e57c96e142b5533 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 17:23:03 +0900 Subject: [PATCH 19/20] chore: readthedocs by trial and error --- .readthedocs.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.readthedocs.yaml b/.readthedocs.yaml index f51d87e3..d573c32b 100644 --- a/.readthedocs.yaml +++ b/.readthedocs.yaml @@ -11,7 +11,7 @@ python: build: os: ubuntu-24.04 tools: - python: "3.13" + python: "3.12" sphinx: configuration: docs/conf.py From 39ca34c703a1cf3f0a7407e70c6fb3acac36dc1f Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Fri, 22 Nov 2024 17:25:36 +0900 Subject: [PATCH 20/20] chore: note the sphinx issue --- .readthedocs.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.readthedocs.yaml b/.readthedocs.yaml index d573c32b..4da2dbd7 100644 --- a/.readthedocs.yaml +++ b/.readthedocs.yaml @@ -11,6 +11,8 @@ python: build: os: ubuntu-24.04 tools: + # Older Shpinx uses imghdr that was removed in Python 3.13 + # See e.g. https://github.com/python/cpython/issues/104818 python: "3.12" sphinx: