From a322f3751181f0f10972c78b60839811121c2e0e Mon Sep 17 00:00:00 2001 From: Simon Gerber Date: Tue, 19 Jul 2022 16:46:23 +0200 Subject: [PATCH] WIP - Allow users to specify instance versions We add support for specifying instance versions in `parameters.components`. Commodore falls back to the version/url/path specified for the component when the keys are not provided for component aliases. Note that the actual dependency handling doesn't yet support overriding URL/path for aliases. --- commodore/compile.py | 2 +- commodore/component/__init__.py | 4 ++ commodore/dependency_mgmt/__init__.py | 26 ++++++--- commodore/dependency_mgmt/version_parsing.py | 60 +++++++++++++++++--- 4 files changed, 76 insertions(+), 16 deletions(-) diff --git a/commodore/compile.py b/commodore/compile.py index e877d199a..820ba65c6 100644 --- a/commodore/compile.py +++ b/commodore/compile.py @@ -194,7 +194,7 @@ def setup_compile_environment(config: Config) -> tuple[dict[str, Any], Iterable[ config.register_component_deprecations(cluster_parameters) # Raise exception if component version override without URL is present in the # hierarchy. - verify_version_overrides(cluster_parameters) + verify_version_overrides(cluster_parameters, config.get_component_aliases()) for component in config.get_components().values(): ckey = component.parameters_key diff --git a/commodore/component/__init__.py b/commodore/component/__init__.py index 1ba6dc7d7..7e4964232 100644 --- a/commodore/component/__init__.py +++ b/commodore/component/__init__.py @@ -89,6 +89,10 @@ def version(self, version: str): def repo_directory(self) -> P: return self._dir + @property + def sub_path(self) -> str: + return self._sub_path + @property def target_directory(self) -> P: return self.alias_directory(self.name) diff --git a/commodore/dependency_mgmt/__init__.py b/commodore/dependency_mgmt/__init__.py index a35157408..4faa2d131 100644 --- a/commodore/dependency_mgmt/__init__.py +++ b/commodore/dependency_mgmt/__init__.py @@ -88,7 +88,7 @@ def fetch_components(cfg: Config): component_names, component_aliases = _discover_components(cfg) click.secho("Registering component aliases...", bold=True) cfg.register_component_aliases(component_aliases) - cspecs = _read_components(cfg, component_names) + cspecs = _read_components(cfg, component_aliases) click.secho("Fetching components...", bold=True) for cn in component_names: cspec = cspecs[cn] @@ -114,8 +114,14 @@ def fetch_components(cfg: Config): continue c = components[component] - # TODO: Set alias version instead of component version - c.register_alias(alias, c.version or "") + aspec = cspecs[alias] + if aspec.url != c.repo_url or aspec.path != c._sub_path: + # TODO: Figure out how we'll handle URL/subpath overrides + raise NotImplementedError( + "URL/path override for component alias not supported" + ) + print(alias, aspec) + c.register_alias(alias, aspec.version) c.checkout_alias(alias) create_alias_symlinks(cfg, c, alias) @@ -131,7 +137,7 @@ def register_components(cfg: Config): click.secho("Discovering included components...", bold=True) try: components, component_aliases = _discover_components(cfg) - cspecs = _read_components(cfg, components) + cspecs = _read_components(cfg, component_aliases) except KeyError as e: raise click.ClickException(f"While discovering components: {e}") click.secho("Registering components and aliases...", bold=True) @@ -174,8 +180,10 @@ def register_components(cfg: Config): continue c = registered_components[cn] - # TODO: Set alias version - c.register_alias(alias, c.version) + aspec = cspecs[alias] + if aspec.url != c.repo_url or aspec.path != c.sub_path: + raise NotImplementedError("Changing alias sub path / URL NYI") + c.register_alias(alias, aspec.version) if not component_dir(cfg.work_dir, alias).is_dir(): raise click.ClickException(f"Missing alias checkout for '{alias} as {cn}'") @@ -238,9 +246,13 @@ def register_packages(cfg: Config): create_package_symlink(cfg, p, pkg) -def verify_version_overrides(cluster_parameters): +def verify_version_overrides(cluster_parameters, component_aliases: dict[str, str]): errors = [] + aliases = set(component_aliases.keys()) - set(component_aliases.values()) for cname, cspec in cluster_parameters["components"].items(): + if cname in aliases: + # We don't require an url in component alias version configs + continue if "url" not in cspec: errors.append(f"component '{cname}'") diff --git a/commodore/dependency_mgmt/version_parsing.py b/commodore/dependency_mgmt/version_parsing.py index ed475322e..a30d1f0a8 100644 --- a/commodore/dependency_mgmt/version_parsing.py +++ b/commodore/dependency_mgmt/version_parsing.py @@ -2,6 +2,7 @@ from collections.abc import Iterable from dataclasses import dataclass +from typing import Optional from enum import Enum @@ -33,18 +34,31 @@ class DependencySpec: path: str @classmethod - def parse(cls, info: dict[str, str]) -> DependencySpec: - if "url" not in info: + def parse( + cls, + info: dict[str, str], + base_config: Optional[DependencySpec] = None, + ) -> DependencySpec: + if "url" not in info and not base_config: raise DependencyParseError("url") - if "version" not in info: + if "version" not in info and not base_config: raise DependencyParseError("version") path = info.get("path", "") if path.startswith("/"): path = path[1:] - return DependencySpec(info["url"], info["version"], path) + if base_config: + url = info.get("url", base_config.url) + version = info.get("version", base_config.version) + if path not in info: + path = base_config.path + else: + url = info["url"] + version = info["version"] + + return DependencySpec(url, version, path) def _read_versions( @@ -53,6 +67,8 @@ def _read_versions( dependency_names: Iterable[str], require_key: bool = True, ignore_class_notfound: bool = False, + aliases: dict[str, str] = {}, + fallback: dict[str, DependencySpec] = {}, ) -> dict[str, DependencySpec]: deps_key = dependency_type.value deptype_str = dependency_type.name.lower() @@ -71,15 +87,26 @@ def _read_versions( # just set deps to the empty dict. deps = {} + if aliases: + all_dep_keys = set(aliases.keys()) + else: + all_dep_keys = deps.keys() for depname in dependency_names: - if depname not in deps: + if depname not in all_dep_keys: raise click.ClickException( f"Unknown {deptype_str} '{depname}'." + f" Please add it to 'parameters.{deps_key}'" ) try: - dep = DependencySpec.parse(deps[depname]) + basename_for_dep = aliases.get(depname, depname) + print(depname, basename_for_dep) + print(deps.get(depname, {})) + print(fallback.get(basename_for_dep)) + dep = DependencySpec.parse( + deps.get(depname, {}), + base_config=fallback.get(basename_for_dep), + ) except DependencyParseError as e: raise click.ClickException( f"{deptype_cap} '{depname}' is missing field '{e.field}'" @@ -96,9 +123,26 @@ def _read_versions( def _read_components( - cfg: Config, component_names: Iterable[str] + cfg: Config, component_aliases: dict[str, str] ) -> dict[str, DependencySpec]: - return _read_versions(cfg, DepType.COMPONENT, component_names) + component_names = set(component_aliases.values()) + alias_names = set(component_aliases.keys()) - component_names + + component_versions = _read_versions(cfg, DepType.COMPONENT, component_names) + alias_versions = _read_versions( + cfg, + DepType.COMPONENT, + alias_names, + aliases=component_aliases, + fallback=component_versions, + ) + + for alias, aspec in alias_versions.items(): + if alias in component_versions: + raise ValueError("alias name already in component_versions?") + component_versions[alias] = aspec + + return component_versions def _read_packages(