fix: apply regex patterns to pydantic fields right (#339)

* fix: apply regex patterns to pydantic fields right Pattern should match the **entire** string Signed-off-by: Vassilis Vassiladis <[email protected]> * chore: tests should display the entire error message Signed-off-by: Vassilis Vassiladis <[email protected]> --------- Signed-off-by: Vassilis Vassiladis <[email protected]>
st4sd · Jun 17, 2024 · a49f425 · a49f425
1 parent 1858e07
commit a49f425
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 15 deletions.
diff --git a/python/experiment/model/frontends/dsl.py b/python/experiment/model/frontends/dsl.py
@@ -130,15 +130,15 @@
 RevampedReferencePattern = fr'"(?P<reference>([.a-zA-Z0-9_/-])+)"{PatternReferenceMethod}'
 LegacyReferencePattern = fr'(?P<reference>([.a-zA-Z0-9_/-])+){PatternReferenceMethod}'
 
-TargetReference = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=fr"<{StepNamePattern}>")]
-ParameterReference = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=ParameterPattern)]
+TargetReference = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=fr"^<{StepNamePattern}>$")]
+ParameterReference = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=fr"^{ParameterPattern}$")]
 MaxRestarts = typing_extensions.Annotated[int, pydantic.Field(gt=-2)]
-BackendType = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=fr'({ParameterPattern}|docker|local|lsf|kubernetes)')]
-K8sQosType = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=fr'({ParameterPattern}|guaranteed|burstable|besteffort)')]
-DockerImagePullPolicy = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=fr'({ParameterPattern}|Always|Never|IfNotPresent)')]
+BackendType = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=fr'^({ParameterPattern}|docker|local|lsf|kubernetes)$')]
+K8sQosType = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=fr'^({ParameterPattern}|guaranteed|burstable|besteffort)$')]
+DockerImagePullPolicy = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=fr'^({ParameterPattern}|Always|Never|IfNotPresent)$')]
 ResourceRequestFloat = typing_extensions.Annotated[int, pydantic.Field(ge=0)]
 ResourceRequestInt = typing_extensions.Annotated[int, pydantic.Field(ge=0)]
-EnvironmentReference = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=fr'({ParameterPattern}|none|environment)')]
+EnvironmentReference = typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=fr'^({ParameterPattern}|none|environment)$')]
 
 
 class OutputReference:
@@ -284,7 +284,7 @@ class Config:
         description="The name of the template, must be unique in the parent namespace",
         min_length=1,
         # VV: Names cannot end in digits - FlowIR has a special meaning for digits at the end of component names
-        pattern=SignatureNamePattern
+        pattern=f"^{SignatureNamePattern}$"
     )
 
     description: typing.Optional[str] = pydantic.Field(
@@ -310,7 +310,7 @@ class Config:
 
     target: TargetReference = pydantic.Field(
         description="Reference to a step name. A string enclosed in <> e.g. <foo>", min_length=3,
-        pattern=fr"<{StepNamePattern}>",
+        pattern=fr"^<{StepNamePattern}>$",
     )
 
     args: typing.Dict[str, ParameterValueType] = pydantic.Field(
@@ -338,7 +338,11 @@ class Config:
         description="The Signature of the Workflow template"
     )
 
-    steps: typing.Dict[str, typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=SignatureNamePattern)]] = pydantic.Field(
+    steps: typing.Dict[
+        str,
+        typing_extensions.Annotated[str, pydantic.StringConstraints(pattern=f"^{SignatureNamePattern}$")
+        ]
+    ] = pydantic.Field(
         description="Instantiated Templates that execute as steps of the parent workflow. "
                     "key: value pairs where the key is the name of the Instance and the value is the name "
                     "of the Template from which to create the Instance."

diff --git a/tests/test_dsl.py b/tests/test_dsl.py
@@ -4,6 +4,7 @@
 
 import typing
 
+import pydantic
 import pytest
 import yaml
 
@@ -1463,4 +1464,60 @@ def test_lightweight_validation_missing_exec():
             "loc": ["workflows", 0, "signature", "parameters", 1],
             "msg": 'The parameter some-parameter in location entry-instance does not have a value or default'
         }
-    ]
+    ]
+
+
+def test_no_replica_names():
+    dsl = {
+        "entrypoint": {
+            "entry-instance": "vv-test",
+            "execute": [{"target": "<entry-instance>"}]
+        },
+        "workflows": [
+            {
+                "signature": {"name": "vv-test"},
+                "steps": {"echo2": "echo2"},
+                "execute": [{"target": "<echo2>"}]
+            }
+        ],
+        "components": [
+            {
+                "signature": {
+                    "name": "echo2",
+                    "parameters": [{"name": "message", "default": "hello world"}]
+                },
+                "command": {"executable": "echo2", "arguments": "%(message)s"}
+            }
+        ]
+    }
+
+
+
+    with pytest.raises(pydantic.ValidationError) as e:
+        _ = experiment.model.frontends.dsl.Namespace(**dsl)
+
+    interesting_keys = ("type", "loc", "msg", "input", "ctx")
+    errors = [
+        {
+            k: v for (k, v) in x.items() if k in interesting_keys
+        } for x in sorted(e.value.errors(), key=lambda x: x["loc"])
+    ]
+
+    assert errors == [
+        {
+            'type': 'string_pattern_mismatch',
+            'loc': ('components', 0, 'signature', 'name'),
+            'msg': "String should match pattern "
+                   "'^(stage(?P<stage>([0-9]+))\\.)?(?P<name>([A-Za-z0-9._-]*[A-Za-z_-]+))$'",
+            'input': 'echo2',
+            'ctx': {'pattern': '^(stage(?P<stage>([0-9]+))\\.)?(?P<name>([A-Za-z0-9._-]*[A-Za-z_-]+))$'},
+        },
+        {
+            'type': 'string_pattern_mismatch',
+            'loc': ('workflows', 0, 'steps', 'echo2'),
+            'msg': "String should match pattern "
+                   "'^(stage(?P<stage>([0-9]+))\\.)?(?P<name>([A-Za-z0-9._-]*[A-Za-z_-]+))$'",
+            'input': 'echo2',
+            'ctx': {'pattern': '^(stage(?P<stage>([0-9]+))\\.)?(?P<name>([A-Za-z0-9._-]*[A-Za-z_-]+))$'},
+        },
+    ]
diff --git a/tox.ini b/tox.ini
@@ -62,14 +62,14 @@ commands=
     {test,testA}-ibm: epatch.py -s0 -e CAFParameterisation.package stage2.DetermineShape...stage2.Montage --platform=paragon -x
 
     # Unit-tests
-    test: pytest  --log-level=10 -n8  tests/
+    test: pytest -vv  --log-level=10 -n8  tests/
 
-    # Same unit-tests but split 3-ways to enable runnin them in parallel Travis jobs
-    testA: pytest --timeout=240 --log-level=10 -n8 --ignore-glob=*test_control.py --ignore-glob=*test_engines.py tests/ --durations=0
+    # Same unit-tests but split 3-ways to enable running them in parallel Travis jobs
+    testA: pytest -vv --timeout=240 --log-level=10 -n8 --ignore-glob=*test_control.py --ignore-glob=*test_engines.py tests/ --durations=0
 
-    testB: pytest --timeout=240 --log-level=10 -n8 tests/test_control.py --durations=0
+    testB: pytest -vv --timeout=240 --log-level=10 -n8 tests/test_control.py --durations=0
 
-    testC: pytest --timeout=240 --log-level=10 -n8 tests/test_engines.py --durations=0
+    testC: pytest -vv --timeout=240 --log-level=10 -n8 tests/test_engines.py --durations=0
 
     {test,testC}-ibm: git clone [email protected]:hartreechem/Synthetic.package.git 
     {test,testC}-ibm: elaunch.py --nostamp Synthetic.package