feat: Allow job parameters as JSON string (#34)

Problem: We currently allow job parameters to be provided as either KEY=VALUE pairs or a json/yaml file. However, there are use-cases where one might have a JSON string but not want to write it to a temp file just to pass it to the CLI. Solution: I've added the option to provide a JSON string as the argument for a job parameter as well. Signed-off-by: Daniel Neilson <[email protected]>
OpenJobDescription · Feb 9, 2024 · 8708b2c · 8708b2c
1 parent 9b278e2
commit 8708b2c
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 12 deletions.
diff --git a/README.md b/README.md
@@ -58,15 +58,15 @@ Displays summary information about a sample Job or a Step therein. The user may
 |Name|Type|Required|Description|Example|
 |---|---|---|---|---|
 |`path`|path|yes|A path leading to a Job template file.|`/path/to/job.template.json`|
-|`--job-param`, `-p`|string, path|no|A key-value pair representing a parameter in the template and the value to use for it, provided as a string or a path to a JSON/YAML document prefixed with 'file://'. Can be specified multiple times.|`--job-param MyParam=5`, `-p file://parameter_file.json`|
+|`--job-param`, `-p`|string, path|no|The values for the job template's parameters. Can be provided as key-value pairs, inline JSON string, or a path to a JSON or YAML document. If provided more than once then the given values are combined in the order that they appear.|`--job-param MyParam=5`, `-p file://parameter_file.json`, `-p '{"MyParam": "5"}'`|
 |`--step-name`|string|no|The name of the Step to summarize.|`--step-name Step1`|
 |`--output`|string|no|How to display the results of the command. Allowed values are `human-readable` (default), `json`, and `yaml`.|`--output json`, `--output yaml`|
 
 #### Example
 ```sh
 $ openjd-cli summary /path/to/job.template.json \
     --job-param JobName=SampleJob \
-    --job-param FileToRender=sample.blend \
+    --job-param '{"FileToRender": "sample.blend"}' \
     --job-param file://some_more_parameters.json
 
 --- Summary for 'SampleJob' ---
@@ -106,7 +106,7 @@ details on how Open Job Description's Jobs are run within Sessions.
 |`path`|path|yes|A path leading to a Job template file.|`/path/to/job.template.json`|
 |`--step-name`|string|yes|The name of the Step to run in a local Session.|`--step-name Step1`|
 |`--environment`|paths|no|Path to a file containing Environment Template definitions. Can be provided multiple times.|`--environment /path/to/env.template1.json --environment /path/to/env.template2.yaml`|
-|`--job-param`, `-p`|string, path|no|A key-value pair representing a parameter in the template and the value to use for it, provided as a string or a path to a JSON/YAML document prefixed with 'file://'. Can be specified multiple times.|`--job-param MyParam=5`, `-p file://parameter_file.json`|
+|`--job-param`, `-p`|string, path|no|The values for the job template's parameters. Can be provided as key-value pairs, inline JSON string, or a path to a JSON or YAML document. If provided more than once then the given values are combined in the order that they appear.|`--job-param MyParam=5`, `-p file://parameter_file.json`, `-p '{"MyParam": "5"}'`|
 |`--task-params`, `-tp`|string, path|no|A list of key-value pairs representing a Task parameter set for the Step, provided as a string or a path to a JSON/YAML document prefixed with 'file://'. If present, the Session will run one Task per parameter set supplied with `--task-params`. Can be specified multiple times.|`--task-params PingCount=20 PingDelay=30`, `-tp file://parameter_set_file.json`|
 |`--maximum-tasks`|integer|no|A maximum number of Tasks to run from this Step. Unless present, the Session will run all Tasks defined in the Step's parameter space, or one Task per `--task-params` argument.|`--maximum-tasks 5`|
 |`--run-dependencies`|flag|no|If present, runs all of a Step's dependencies in the Session prior to the Step itself.|`--run-dependencies`|

diff --git a/src/openjd/cli/_common/__init__.py b/src/openjd/cli/_common/__init__.py
@@ -77,8 +77,12 @@ def add_common_arguments(
             dest="job_params",
             type=str,
             action="append",
-            metavar=("KEY=VALUE, file://PATH_TO_PARAMS"),
-            help="Use these Job parameters with the provided template. Can be provided as key-value pairs, or as path(s) to a JSON or YAML document prefixed with 'file://'.",
+            metavar=('KEY=VALUE, file://PATH_TO_PARAMS, \'{"KEY": "VALUE", ... }\''),
+            help=(
+                "Use these Job parameters with the provided template. Can be provided as key-value pairs, "
+                "path(s) to a JSON or YAML document prefixed with 'file://', or inline JSON. If this option "
+                "is provided more than once then the given values are all combined in the order that they appear."
+            ),
         )
 
 

diff --git a/src/openjd/cli/_common/_job_from_template.py b/src/openjd/cli/_common/_job_from_template.py
@@ -63,6 +63,7 @@ def get_job_params(parameter_args: list[str]) -> dict:
     """
     parameter_dict: dict = {}
     for arg in parameter_args:
+        arg = arg.strip()
         # Case 1: Provided argument is a filepath
         if arg.startswith("file://"):
             # Raises: RuntimeError
@@ -73,14 +74,31 @@ def get_job_params(parameter_args: list[str]) -> dict:
             else:
                 raise RuntimeError(f"Job parameter file '{arg}' should contain a dictionary.")
 
-        # Case 2: Provided argument is a Key=Value string
-        else:
-            regex_match = re.match("(.+)=(.*)", arg)
+        # Case 2: Provided as a JSON string
+        elif re.match("^{(.*)}$", arg):
+            try:
+                # Raises: JSONDecodeError
+                parameters = json.loads(arg)
+            except (json.JSONDecodeError, TypeError):
+                raise RuntimeError(
+                    f"Job parameter string ('{arg}') not formatted correctly. It must be key=value pairs, inline JSON, or a path to a JSON or YAML document prefixed with 'file://'."
+                )
+            if not isinstance(parameters, dict):
+                # This should never happen. Including it out of a sense of paranoia.
+                raise RuntimeError(
+                    f"Job parameter ('{arg}') must contain a dictionary mapping job parameters to their value."
+                )
+            parameter_dict.update(parameters)
 
-            if not regex_match:
-                raise RuntimeError(f"Job parameter '{arg}' should be in the format 'Key=Value'.")
+        # Case 3: Provided argument is a Key=Value string
+        elif regex_match := re.match("^(.+)=(.*)$", arg):
             parameter_dict.update({regex_match[1]: regex_match[2]})
 
+        else:
+            raise RuntimeError(
+                f"Job parameter string ('{arg}') not formatted correctly. It must be key=value pairs, inline JSON, or a path to a JSON or YAML document prefixed with 'file://'."
+            )
+
     return parameter_dict
 
 

diff --git a/test/openjd/cli/test_common.py b/test/openjd/cli/test_common.py
@@ -184,6 +184,14 @@ def test_read_environment_template_parsingerror(tempfile_extension: str, file_co
     [
         pytest.param(MOCK_PARAM_ARGUMENTS, MOCK_PARAM_VALUES, id="Params from key-value pair"),
         pytest.param(["file://TEMPDIR/params.json"], MOCK_PARAM_VALUES, id="Params from file"),
+        pytest.param(
+            [json.dumps({"MyParam": "5"})], {"MyParam": "5"}, id="Params from json string"
+        ),
+        pytest.param(
+            [json.dumps({"MyParam": "Value=5"})],
+            {"MyParam": "Value=5"},
+            id="Params from json string",
+        ),
         pytest.param(
             ["SomeParam=SomeValue", "file://TEMPDIR/params.json"],
             {"SomeParam": "SomeValue", "Title": "overwrite", "RequiredParam": "5"},
@@ -220,7 +228,7 @@ def test_get_job_params_success(mock_param_args: list[str], expected_param_value
             False,
             "",
             None,
-            "'bad format' should be in the format 'Key=Value'",
+            "Job parameter string ('bad format') not formatted correctly.",
             id="Badly-formatted parameter string",
         ),
         pytest.param(
@@ -272,7 +280,7 @@ def test_get_job_params_success(mock_param_args: list[str], expected_param_value
             ["file://bad-params.yaml"],
             True,
             True,
-            "bad-params.yaml",
+            "bad-params.json",
             lambda: '"bad":\n"yaml"',
             "is formatted incorrectly",
             id="Badly-formatted parameter file (YAML)",
@@ -286,6 +294,24 @@ def test_get_job_params_success(mock_param_args: list[str], expected_param_value
             "should contain a dictionary",
             id="Non-dictionary file contents",
         ),
+        pytest.param(
+            ["- not json -"],
+            False,
+            False,
+            "",
+            None,
+            "Job parameter string ('- not json -') not formatted correctly.",
+            id="Not JSON",
+        ),
+        pytest.param(
+            ['["a", "b"]'],
+            False,
+            False,
+            "",
+            None,
+            'Job parameter string (\'["a", "b"]\') not formatted correctly.',
+            id="JSON not dictionary",
+        ),
     ],
 )
 def test_get_job_params_error(