Skip to content

Commit

Permalink
Validate the form of the image versions.
Browse files Browse the repository at this point in the history
Previously, we just required any value.

Now, we require either `latest`, or someing in the form `vN[.N]+`.

This will help our users detect typos earlier.

Fixes #291
  • Loading branch information
bloodearnest committed Jan 21, 2025
1 parent 4bda291 commit 7670bf0
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 6 deletions.
9 changes: 7 additions & 2 deletions pipeline/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,9 @@ def build(

return action

# Valid image versions. Note: at some point, we probably want to disallow latest.
VERSION_REGEX = re.compile(r"^((v[\d.]+)|latest)$")

@classmethod
def parse_run_string(cls, action_id: str, run: str) -> Command:
if run == "":
Expand All @@ -224,9 +227,11 @@ def parse_run_string(cls, action_id: str, run: str) -> Command:
parts = shlex.split(run)

name, _, version = parts[0].partition(":")
if not version:

vmatch = cls.VERSION_REGEX.match(version)
if not vmatch:
raise ValidationError(
f"{name} must have a version specified (e.g. {name}:0.5.2)",
f"Action command {name} must have a version specified in the form :vN (e.g. {name}:v2)",
)

return Command(raw=run)
Expand Down
47 changes: 43 additions & 4 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,23 @@ def test_success():
Pipeline.build(**data)


def test_action_has_a_version():
@pytest.mark.parametrize(
"action",
[
"test",
"test:",
"test:v",
"test:other",
"test:vnotdigits",
"test:v1x1",
],
)
def test_action_handles_invalid_version(action):
data = {
"version": 1,
"actions": {
"generate_cohort": {
"run": "test foo",
"run": action,
"outputs": {
"highly_sensitive": {"cohort": "output/input.csv"},
},
Expand All @@ -39,6 +50,34 @@ def test_action_has_a_version():
Pipeline.build(**data)


@pytest.mark.parametrize(
"action",
[
"test:v1",
"test:v2",
"test:v1.2",
"test:v1.2.3",
],
)
def test_action_handles_valid_version(action):
data = {
"version": 1,
"actions": {
"generate_cohort": {
"run": action,
"outputs": {
"highly_sensitive": {"cohort": "output/input.csv"},
},
}
},
}

run = Pipeline.build(**data).actions["generate_cohort"].run
n, _, v = action.partition(":")
assert run.name == n
assert run.version == v


def test_action_cohortextractor_multiple_outputs_with_output_flag():
data = {
"version": 1,
Expand Down Expand Up @@ -337,13 +376,13 @@ def test_pipeline_with_duplicated_action_run_commands():
"version": 1,
"actions": {
"action1": {
"run": "test:lastest",
"run": "test:latest",
"outputs": {
"moderately_sensitive": {"cohort": "output.csv"},
},
},
"action2": {
"run": "test:lastest",
"run": "test:latest",
"outputs": {
"moderately_sensitive": {"cohort": "output.csv"},
},
Expand Down

0 comments on commit 7670bf0

Please sign in to comment.