From 364082be9eeb14204aeebee2b2d7c0dd956f3183 Mon Sep 17 00:00:00 2001 From: James Garner Date: Fri, 18 Oct 2024 14:29:20 +1300 Subject: [PATCH 1/3] fix: don't use lexical sorting for version numbers in codegen Previously, python-libjuju iterated over schemas keyed by their version string (e.g. '3.1.9') using lexical sorting. For a given facade version, a definition in a higher versioned schema was intended to overwrite any prior definition saved (see generate_facades function in facade.py). With lexical sorting, '3.1.10' would be sorted in between '3.1.1' and '3.1.2', which would not lead to the desired behaviour. This commit fixes this problem by using a tuple of integers as the sorting key. A special case is requried for the version string 'latest', and we use (9000, 9000, 9000). --- juju/client/facade.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/juju/client/facade.py b/juju/client/facade.py index 9a8511c5d..854ff43cd 100644 --- a/juju/client/facade.py +++ b/juju/client/facade.py @@ -13,7 +13,7 @@ from collections import defaultdict from glob import glob from pathlib import Path -from typing import Any, Mapping, Sequence, TypeVar +from typing import Any, Dict, List, Mapping, Sequence, Tuple, TypeVar import typing_inspect @@ -926,11 +926,22 @@ def generate_definitions(schemas): return definitions -def generate_facades(schemas): +def sortable_schema_version(version_string: str) -> Tuple[int, int, int]: + """Return a sorting key in the form (major, minor, micro) from a version string.""" + # 'latest' is special cased in load_schemas and should come last + if version_string == 'latest': + return (9000, 9000, 9000) + # raise ValueError if string isn't in the format A.B.C + major, minor, micro = version_string.split('.') + # raise ValueError if major, minor, and micro aren't int strings + return (int(major), int(minor), int(micro)) + + +def generate_facades(schemas: Dict[str, List[Schema]]) -> Dict[str, Dict[int, codegen.Capture]]: captures = defaultdict(codegen.Capture) # Build the Facade classes - for juju_version in sorted(schemas.keys()): + for juju_version in sorted(schemas.keys(), key=sortable_schema_version): for schema in schemas[juju_version]: cls, source = buildFacade(schema) cls_name = "{}Facade".format(schema.name) From 84ccfe8f1979f39a06b6642ce6fb24ded667a3b6 Mon Sep 17 00:00:00 2001 From: Dima Tisnek Date: Tue, 22 Oct 2024 15:21:17 +0900 Subject: [PATCH 2/3] chore: simpler code --- juju/client/facade.py | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/juju/client/facade.py b/juju/client/facade.py index 854ff43cd..e954c497a 100644 --- a/juju/client/facade.py +++ b/juju/client/facade.py @@ -13,8 +13,9 @@ from collections import defaultdict from glob import glob from pathlib import Path -from typing import Any, Dict, List, Mapping, Sequence, Tuple, TypeVar +from typing import Any, Dict, List, Mapping, Sequence +import packaging.version import typing_inspect from . import codegen @@ -150,7 +151,7 @@ def get(self, name): # Two way mapping refname = self.schema.referenceName(name) if refname not in self: - result = TypeVar(refname) + result = typing.TypeVar(refname) self[refname] = result self[result] = refname @@ -926,22 +927,11 @@ def generate_definitions(schemas): return definitions -def sortable_schema_version(version_string: str) -> Tuple[int, int, int]: - """Return a sorting key in the form (major, minor, micro) from a version string.""" - # 'latest' is special cased in load_schemas and should come last - if version_string == 'latest': - return (9000, 9000, 9000) - # raise ValueError if string isn't in the format A.B.C - major, minor, micro = version_string.split('.') - # raise ValueError if major, minor, and micro aren't int strings - return (int(major), int(minor), int(micro)) - - def generate_facades(schemas: Dict[str, List[Schema]]) -> Dict[str, Dict[int, codegen.Capture]]: captures = defaultdict(codegen.Capture) # Build the Facade classes - for juju_version in sorted(schemas.keys(), key=sortable_schema_version): + for juju_version in sorted(schemas.keys(), key=packaging.version.parse): for schema in schemas[juju_version]: cls, source = buildFacade(schema) cls_name = "{}Facade".format(schema.name) From b2f118b1d949b25ec5bf5070b7250f674f6ef4ca Mon Sep 17 00:00:00 2001 From: James Garner Date: Mon, 28 Oct 2024 11:13:11 +0100 Subject: [PATCH 3/3] chore: remove special casing of 'latest' string when loading schemas --- juju/client/facade.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/juju/client/facade.py b/juju/client/facade.py index e954c497a..00e7f5310 100644 --- a/juju/client/facade.py +++ b/juju/client/facade.py @@ -954,18 +954,13 @@ def generate_facades(schemas: Dict[str, List[Schema]]) -> Dict[str, Dict[int, co def load_schemas(options): schemas = {} - for p in sorted(glob(options.schema)): - if 'latest' in p: - juju_version = 'latest' - else: - try: - juju_version = re.search(JUJU_VERSION, p).group() - except AttributeError: - print("Cannot extract a juju version from {}".format(p)) - print("Schemas must include a juju version in the filename") - raise SystemExit(1) - + try: + juju_version = re.search(JUJU_VERSION, p).group() + except AttributeError: + print("Cannot extract a juju version from {}".format(p)) + print("Schemas must include a juju version in the filename") + raise SystemExit(1) new_schemas = json.loads(Path(p).read_text("utf-8")) schemas[juju_version] = [Schema(s) for s in new_schemas] return schemas