Skip to content

Commit

Permalink
WIP - Allow users to specify instance versions
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
simu committed Jul 19, 2022
1 parent a1c62d0 commit a322f37
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 16 deletions.
2 changes: 1 addition & 1 deletion commodore/compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions commodore/component/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
26 changes: 19 additions & 7 deletions commodore/dependency_mgmt/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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}'")

Expand Down Expand Up @@ -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}'")

Expand Down
60 changes: 52 additions & 8 deletions commodore/dependency_mgmt/version_parsing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from collections.abc import Iterable
from dataclasses import dataclass
from typing import Optional

from enum import Enum

Expand Down Expand Up @@ -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(
Expand All @@ -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()
Expand All @@ -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}'"
Expand All @@ -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(
Expand Down

0 comments on commit a322f37

Please sign in to comment.