Skip to content

Commit a20fe4e

Browse files
committed
Rough Draft of support for field test_version in component spec
Commodore will first try to checkout `test_version` for components who set the field, and fall back to field `version` if the version given in `test_version` can't be checked out and isn't equal to `version`. This feature might be useful on development or Lab clusters where many users test various component feature branches, and generally the absence of a configured version indicates that the feature branch was merged and the component's main development branch (usually `master`) should be used again. Note that we probably should make this feature opt-in via a cluster fact or field in `parameters.commodore`.
1 parent 7f28670 commit a20fe4e

5 files changed

Lines changed: 41 additions & 22 deletions

File tree

commodore/dependency_mgmt/__init__.py

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,16 @@ def fetch_components(cfg: Config):
103103
cn,
104104
work_dir=cfg.work_dir,
105105
dependency=cdep,
106-
version=cspec.version,
106+
version=cspec.test_version or cspec.version,
107107
sub_path=cspec.path,
108108
)
109109
if c.checkout_is_dirty() and not cfg.force:
110110
raise click.ClickException(
111111
f"Component {cn} has uncommitted changes. "
112112
+ "Please specify `--force` to discard them"
113113
)
114-
deps.setdefault(cdep.url, []).append(c)
114+
ci = (c, cspec.version)
115+
deps.setdefault(cdep.url, []).append(ci)
115116
do_parallel(fetch_component, cfg, deps.values())
116117

117118
_setup_component_aliases(cfg, component_aliases, cspecs, set(deps.keys()))
@@ -155,15 +156,26 @@ def _setup_component_aliases(
155156
do_parallel(setup_alias, cfg, aliases.values())
156157

157158

158-
def fetch_component(cfg: Config, dependencies: Iterable[Component]):
159+
def fetch_component(cfg: Config, dependencies: Iterable[tuple[Component, str]]):
159160
"""
160161
Fetch all components of a MultiDependency object.
161162
"""
162-
for c in dependencies:
163+
for c, fallback_version in dependencies:
163164
try:
164165
c.checkout()
165166
except RefError as e:
166-
raise ClickException(f"while fetching component {c.name}: {e}")
167+
if c.version == fallback_version:
168+
raise ClickException(f"while fetching component {c.name}: {e}")
169+
else:
170+
click.echo(
171+
f" > Unable to checkout version '{c.version}' for component '{c.name}', "
172+
+ f"trying again with version '{fallback_version}'"
173+
)
174+
c.version = fallback_version
175+
try:
176+
c.checkout()
177+
except RefError as e:
178+
raise ClickException(f"while fetching component {c.name}: {e}")
167179
cfg.register_component(c)
168180
create_component_symlinks(cfg, c)
169181

commodore/dependency_mgmt/version_parsing.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ class DependencySpec:
3131

3232
url: str
3333
version: str
34+
test_version: Optional[str]
3435
path: str
3536

3637
@classmethod
@@ -52,13 +53,15 @@ def parse(
5253
if base_config:
5354
url = info.get("url", base_config.url)
5455
version = info.get("version", base_config.version)
56+
test_version = info.get("test_version", base_config.test_version)
5557
if not path:
5658
path = base_config.path
5759
else:
5860
url = info["url"]
5961
version = info["version"]
62+
test_version = info.get("test_version")
6063

61-
return DependencySpec(url, version, path)
64+
return DependencySpec(url, version, test_version, path)
6265

6366

6467
def _read_versions(
@@ -112,6 +115,10 @@ def _read_versions(
112115
if cfg.debug:
113116
click.echo(f" > URL for {depname}: {dep.url}")
114117
click.echo(f" > Version for {depname}: {dep.version}")
118+
if dep.test_version:
119+
click.echo(
120+
f" > Test (override) version for {depname}: {dep.test_version}"
121+
)
115122
click.echo(f" > Subpath for {depname}: {dep.path}")
116123

117124
dependencies[depname] = dep

commodore/inventory/lint_dependency_specification.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ def lint_dependency_specification(
3030
)
3131
errcount += 1
3232

33-
unk_keys = set(dspec.keys()) - {"url", "version", "path"}
33+
unk_keys = set(dspec.keys()) - {"url", "version", "test_version", "path"}
3434
if len(unk_keys) > 0:
3535
click.secho(
3636
f"> {deptype_str} specification for {d} "

tests/test_dependency_mgmt.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ def _prepare_repository(
7373

7474
repo.index.add([f"{class_path}/defaults.yml", f"{class_path}/{component_name}.yml"])
7575
repo.index.commit("component defaults")
76-
return DependencySpec(url, version, sub_path)
76+
return DependencySpec(url, version, None, sub_path)
7777

7878

7979
def test_create_component_symlinks_fails(config: Config, tmp_path: Path, mockdep):
@@ -365,7 +365,7 @@ def test_register_components(
365365
component_dirs, other_dirs = _setup_register_components(tmp_path)
366366
patch_discover.return_value = (component_dirs, {})
367367
patch_read.return_value = {
368-
cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", "")
368+
cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", None, "")
369369
for cn in component_dirs
370370
}
371371

@@ -389,7 +389,7 @@ def test_register_components_and_aliases(
389389
)
390390
patch_discover.return_value = (component_dirs, alias_data)
391391
patch_read.return_value = {
392-
cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", "")
392+
cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", None, "")
393393
for cn in component_dirs
394394
}
395395
patch_read.return_value["fooer"] = patch_read.return_value["foo"]
@@ -420,7 +420,7 @@ def test_register_components_and_aliases_raises(
420420
component_dirs, other_dirs = _setup_register_components(tmp_path)
421421
patch_discover.return_value = (component_dirs, alias_data)
422422
patch_read.return_value = {
423-
cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", "")
423+
cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", None, "")
424424
for cn in component_dirs
425425
}
426426
patch_read.return_value["fooer"] = patch_read.return_value["foo"]
@@ -441,7 +441,7 @@ def test_register_unknown_components(
441441
component_dirs.extend(unknown_components)
442442
patch_discover.return_value = (component_dirs, {})
443443
patch_read.return_value = {
444-
cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", "")
444+
cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", None, "")
445445
for cn in component_dirs
446446
}
447447

@@ -469,7 +469,7 @@ def test_register_dangling_aliases(
469469

470470
patch_discover.return_value = (component_dirs, alias_data)
471471
patch_read.return_value = {
472-
cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", "")
472+
cn: DependencySpec(f"https://fake.repo.url/{cn}.git", "master", None, "")
473473
for cn in component_dirs
474474
}
475475
patch_read.return_value["bazzer"] = patch_read.return_value["baz"]
@@ -714,7 +714,7 @@ def _setup_packages(
714714
_setup_package_remote(p, upstream_path / f"{p}.git")
715715
url = f"file://{upstream_path}/{p}.git"
716716
version = "master"
717-
package_specs[p] = DependencySpec(url, version, "")
717+
package_specs[p] = DependencySpec(url, version, None, "")
718718

719719
return package_specs
720720

@@ -833,7 +833,7 @@ def test_fetch_component_raises_clickexception(tmp_path: Path, config: Config):
833833
version=cspec.version,
834834
)
835835
with pytest.raises(click.ClickException) as exc:
836-
dependency_mgmt.fetch_component(config, [component])
836+
dependency_mgmt.fetch_component(config, [(component, cspec.version)])
837837

838838
assert (
839839
"while fetching component test-component: Failed to checkout revision 'foo'"

tests/test_dependency_mgmt_version_parsing.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ def test_read_components_exc(
144144
["test"],
145145
{
146146
"test": version_parsing.DependencySpec(
147-
"https://git.example.com/pkg.git", "v1.0.0", ""
147+
"https://git.example.com/pkg.git", "v1.0.0", None, ""
148148
),
149149
},
150150
),
@@ -153,10 +153,10 @@ def test_read_components_exc(
153153
["test", "foo"],
154154
{
155155
"test": version_parsing.DependencySpec(
156-
"https://git.example.com/pkg.git", "v1.0.0", ""
156+
"https://git.example.com/pkg.git", "v1.0.0", None, ""
157157
),
158158
"foo": version_parsing.DependencySpec(
159-
"https://git.example.com/foo.git", "feat/initial", ""
159+
"https://git.example.com/foo.git", "feat/initial", None, ""
160160
),
161161
},
162162
),
@@ -165,16 +165,16 @@ def test_read_components_exc(
165165
["test", "foo", "bar", "baz"],
166166
{
167167
"test": version_parsing.DependencySpec(
168-
"https://git.example.com/pkg.git", "v1.0.0", ""
168+
"https://git.example.com/pkg.git", "v1.0.0", None, ""
169169
),
170170
"foo": version_parsing.DependencySpec(
171-
"https://git.example.com/foo.git", "feat/initial", ""
171+
"https://git.example.com/foo.git", "feat/initial", None, ""
172172
),
173173
"bar": version_parsing.DependencySpec(
174-
"https://git.example.com/barbaz.git", "master", "bar"
174+
"https://git.example.com/barbaz.git", "master", None, "bar"
175175
),
176176
"baz": version_parsing.DependencySpec(
177-
"https://git.example.com/barbaz.git", "master", "baz"
177+
"https://git.example.com/barbaz.git", "master", None, "baz"
178178
),
179179
},
180180
),

0 commit comments

Comments
 (0)