From 86876815b166f94c6388caa122120dbdfd010c5c Mon Sep 17 00:00:00 2001 From: PietroPasotti Date: Wed, 14 Feb 2024 15:30:53 +0100 Subject: [PATCH] general maintenance and scenario tests fixes (#223) * tests green * fix jujudep --- .../v1/alertmanager_dispatch.py | 34 ++-- lib/charms/traefik_k8s/v2/ingress.py | 167 +++++++++++++----- requirements.txt | 5 +- tox.ini | 19 +- 4 files changed, 149 insertions(+), 76 deletions(-) diff --git a/lib/charms/alertmanager_k8s/v1/alertmanager_dispatch.py b/lib/charms/alertmanager_k8s/v1/alertmanager_dispatch.py index cee1efde..d35df0b9 100644 --- a/lib/charms/alertmanager_k8s/v1/alertmanager_dispatch.py +++ b/lib/charms/alertmanager_k8s/v1/alertmanager_dispatch.py @@ -33,6 +33,7 @@ def __init__(self, *args): from ops.charm import CharmBase, RelationEvent, RelationJoinedEvent, RelationRole from ops.framework import EventBase, EventSource, Object, ObjectEvents from ops.model import Relation +from pydantic import computed_field # The unique Charmhub library identifier, never change it LIBID = "37f1ca6f8fe84e3092ebbf6dc2885310" @@ -42,9 +43,9 @@ def __init__(self, *args): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 2 -PYDEPS = ["pydantic"] +PYDEPS = ["pydantic>=2"] # Set to match metadata.yaml INTERFACE_NAME = "alertmanager_dispatch" @@ -61,23 +62,20 @@ class _ProviderSchemaV0(pydantic.BaseModel): class _ProviderSchemaV1(pydantic.BaseModel): url: str - # The following are v0 fields that are continued to be populated for backwards compatibility. - # TODO: when we switch to pydantic 2+, use computed_field instead of the following fields, and - # also drop the __init__. - # https://docs.pydantic.dev/latest/api/fields/#pydantic.fields.computed_field - public_address: Optional[str] # v0 relic - scheme: Optional[str] # v0 relic - - def __init__(self, **kwargs): - super().__init__(**kwargs) - - parsed = urlparse(kwargs["url"]) + @computed_field + @property + def public_address(self) -> Optional[str]: + # v0 relic + parsed = urlparse(self.url) port = ":" + str(parsed.port) if parsed.port else "" - public_address = f"{parsed.hostname}{port}{parsed.path}" + return f"{parsed.hostname}{port}{parsed.path}" - # Derive v0 fields from v1 field - self.public_address = public_address - self.scheme = parsed.scheme + @computed_field + @property + def scheme(self) -> Optional[str]: + # v0 relic + parsed = urlparse(self.url) + return parsed.scheme class ClusterChanged(EventBase): @@ -332,7 +330,7 @@ def _generate_relation_data(self, relation: Relation) -> Dict[str, str]: # "alertmanagers.[].static_configs.targets" section in the prometheus config should list # all units. data = _ProviderSchemaV1(url=self._external_url) - return data.dict() + return data.model_dump() def _update_relation_data(self, event: Optional[RelationEvent] = None): """Helper function for updating relation data bags. diff --git a/lib/charms/traefik_k8s/v2/ingress.py b/lib/charms/traefik_k8s/v2/ingress.py index 31028e97..c4eb15b6 100644 --- a/lib/charms/traefik_k8s/v2/ingress.py +++ b/lib/charms/traefik_k8s/v2/ingress.py @@ -1,4 +1,4 @@ -# Copyright 2023 Canonical Ltd. +# Copyright 2024 Canonical Ltd. # See LICENSE file for licensing details. r"""# Interface Library for ingress. @@ -72,9 +72,9 @@ def _on_ingress_revoked(self, event: IngressPerAppRevokedEvent): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 8 +LIBPATCH = 10 -PYDEPS = ["pydantic<2.0"] +PYDEPS = ["pydantic"] DEFAULT_RELATION_NAME = "ingress" RELATION_INTERFACE = "ingress" @@ -82,59 +82,138 @@ def _on_ingress_revoked(self, event: IngressPerAppRevokedEvent): log = logging.getLogger(__name__) BUILTIN_JUJU_KEYS = {"ingress-address", "private-address", "egress-subnets"} +if int(pydantic.version.VERSION.split(".")[0]) < 2: -class DatabagModel(BaseModel): - """Base databag model.""" + class DatabagModel(BaseModel): # type: ignore + """Base databag model.""" - class Config: - """Pydantic config.""" - - allow_population_by_field_name = True - """Allow instantiating this class by field name (instead of forcing alias).""" + class Config: + """Pydantic config.""" - _NEST_UNDER = None + allow_population_by_field_name = True + """Allow instantiating this class by field name (instead of forcing alias).""" - @classmethod - def load(cls, databag: MutableMapping): - """Load this model from a Juju databag.""" - if cls._NEST_UNDER: - return cls.parse_obj(json.loads(databag[cls._NEST_UNDER])) + _NEST_UNDER = None - try: - data = {k: json.loads(v) for k, v in databag.items() if k not in BUILTIN_JUJU_KEYS} - except json.JSONDecodeError as e: - msg = f"invalid databag contents: expecting json. {databag}" - log.error(msg) - raise DataValidationError(msg) from e + @classmethod + def load(cls, databag: MutableMapping): + """Load this model from a Juju databag.""" + if cls._NEST_UNDER: + return cls.parse_obj(json.loads(databag[cls._NEST_UNDER])) - try: - return cls.parse_raw(json.dumps(data)) # type: ignore - except pydantic.ValidationError as e: - msg = f"failed to validate databag: {databag}" - log.error(msg, exc_info=True) - raise DataValidationError(msg) from e + try: + data = { + k: json.loads(v) + for k, v in databag.items() + # Don't attempt to parse model-external values + if k in {f.alias for f in cls.__fields__.values()} + } + except json.JSONDecodeError as e: + msg = f"invalid databag contents: expecting json. {databag}" + log.error(msg) + raise DataValidationError(msg) from e - def dump(self, databag: Optional[MutableMapping] = None, clear: bool = True): - """Write the contents of this model to Juju databag. + try: + return cls.parse_raw(json.dumps(data)) # type: ignore + except pydantic.ValidationError as e: + msg = f"failed to validate databag: {databag}" + log.debug(msg, exc_info=True) + raise DataValidationError(msg) from e + + def dump(self, databag: Optional[MutableMapping] = None, clear: bool = True): + """Write the contents of this model to Juju databag. + + :param databag: the databag to write the data to. + :param clear: ensure the databag is cleared before writing it. + """ + if clear and databag: + databag.clear() + + if databag is None: + databag = {} + + if self._NEST_UNDER: + databag[self._NEST_UNDER] = self.json(by_alias=True) + return databag + + dct = self.dict() + for key, field in self.__fields__.items(): # type: ignore + value = dct[key] + databag[field.alias or key] = json.dumps(value) + + return databag + +else: + from pydantic import ConfigDict + + class DatabagModel(BaseModel): + """Base databag model.""" + + model_config = ConfigDict( + # tolerate additional keys in databag + extra="ignore", + # Allow instantiating this class by field name (instead of forcing alias). + populate_by_name=True, + # Custom config key: whether to nest the whole datastructure (as json) + # under a field or spread it out at the toplevel. + _NEST_UNDER=None, + ) # type: ignore + """Pydantic config.""" - :param databag: the databag to write the data to. - :param clear: ensure the databag is cleared before writing it. - """ - if clear and databag: - databag.clear() + @classmethod + def load(cls, databag: MutableMapping): + """Load this model from a Juju databag.""" + nest_under = cls.model_config.get("_NEST_UNDER") + if nest_under: + return cls.model_validate(json.loads(databag[nest_under])) # type: ignore - if databag is None: - databag = {} + try: + data = { + k: json.loads(v) + for k, v in databag.items() + # Don't attempt to parse model-external values + if k in {(f.alias or n) for n, f in cls.__fields__.items()} + } + except json.JSONDecodeError as e: + msg = f"invalid databag contents: expecting json. {databag}" + log.error(msg) + raise DataValidationError(msg) from e - if self._NEST_UNDER: - databag[self._NEST_UNDER] = self.json() + try: + return cls.model_validate_json(json.dumps(data)) # type: ignore + except pydantic.ValidationError as e: + msg = f"failed to validate databag: {databag}" + log.debug(msg, exc_info=True) + raise DataValidationError(msg) from e + + def dump(self, databag: Optional[MutableMapping] = None, clear: bool = True): + """Write the contents of this model to Juju databag. + + :param databag: the databag to write the data to. + :param clear: ensure the databag is cleared before writing it. + """ + if clear and databag: + databag.clear() + + if databag is None: + databag = {} + nest_under = self.model_config.get("_NEST_UNDER") + if nest_under: + databag[nest_under] = self.model_dump_json( # type: ignore + by_alias=True, + # skip keys whose values are default + exclude_defaults=True, + ) + return databag - dct = self.dict() - for key, field in self.__fields__.items(): # type: ignore - value = dct[key] - databag[field.alias or key] = json.dumps(value) + dct = self.model_dump() # type: ignore + for key, field in self.model_fields.items(): # type: ignore + value = dct[key] + if value == field.default: + continue + databag[field.alias or key] = json.dumps(value) - return databag + return databag # todo: import these models from charm-relation-interfaces/ingress/v2 instead of redeclaring them diff --git a/requirements.txt b/requirements.txt index 5fd2f139..e407d130 100644 --- a/requirements.txt +++ b/requirements.txt @@ -11,7 +11,7 @@ jsonschema # Code: https://github.com/canonical/operator/ # Docs: https://ops.rtfd.io/ # Deps: charm -ops < 2.5.0 # https://github.com/canonical/ops-scenario/issues/48 +ops >= 2.10 # YAML processing framework # Code: https://github.com/yaml/pyyaml @@ -29,3 +29,6 @@ lightkube-models # Cryptography # Deps: tls_certificates cryptography + +# Deps: traefik.v2.ingress +pydantic>=2 diff --git a/tox.ini b/tox.ini index 8cbb4fba..33559a36 100644 --- a/tox.ini +++ b/tox.ini @@ -53,9 +53,7 @@ commands = description = Run static analysis checks deps = pyright - charm: -r{toxinidir}/requirements.txt - lib: ops - pydantic < 2.0 # from alertmanager_k8s.v1.alertmanager_dispatch + -r{toxinidir}/requirements.txt setenv = PYRIGHT_PYTHON_FORCE_VERSION = latest commands = @@ -86,7 +84,6 @@ deps = hypothesis validators>=0.21.2 -r{toxinidir}/requirements.txt - pydantic < 2.0 # from traefik_k8s.v2.ingress cosl commands = coverage run \ @@ -94,18 +91,13 @@ commands = -m pytest -v --tb native --log-cli-level=INFO -s {posargs} {[vars]tst_path}/unit coverage report -# Added a '-disabled' suffix so CI won't fail on scenario tests, due to -# - https://github.com/canonical/ops-scenario/issues/48 -# - https://github.com/canonical/ops-scenario/issues/49 -[testenv:scenario-disabled] +[testenv:scenario] description = Scenario tests deps = pytest - pydantic < 2 - ops-scenario - ops < 2.5.0 # https://github.com/canonical/ops-scenario/issues/48 + ops-scenario>=6 + cosl -r{toxinidir}/requirements.txt - pydantic < 2.0 # from traefik_k8s.v2.ingress commands = pytest -v --tb native {[vars]tst_path}/scenario --log-cli-level=INFO -s {posargs} @@ -123,7 +115,8 @@ commands = description = Run integration tests deps = deepdiff - juju + # https://github.com/juju/python-libjuju/issues/1025 + juju<=3.3.0,>=3.0 pytest pytest-operator pytest-httpserver