From 8708b2ced5945465fd6706d95eac0bb1ac6317ca Mon Sep 17 00:00:00 2001 From: Daniel Neilson <53624638+ddneilson@users.noreply.github.com> Date: Fri, 9 Feb 2024 11:05:53 -0600 Subject: [PATCH] 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 <53624638+ddneilson@users.noreply.github.com> --- README.md | 6 ++-- src/openjd/cli/_common/__init__.py | 8 ++++-- src/openjd/cli/_common/_job_from_template.py | 28 ++++++++++++++---- test/openjd/cli/test_common.py | 30 ++++++++++++++++++-- 4 files changed, 60 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 4d5516b..adefb4b 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ 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`| @@ -66,7 +66,7 @@ Displays summary information about a sample Job or a Step therein. The user may ```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 index d0058e8..ef7be5b 100644 --- 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 index c0f3317..1ae9044 100644 --- 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 index e38ed75..0922c15 100644 --- 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(