Skip to content

Commit

Permalink
feat!: modifying how tasks are selected in run command
Browse files Browse the repository at this point in the history 
Problem:

The way that tasks were selected for running in the `run` subcommand
was ambiguous. For example `openjd run -tp Foo=Bar template.json -tp
Buz=Baz ...` We allowed multiple values to a single `-tp` argument, and
that makes it less clear whether the 'template.json' in there is
intended to be the positionl argument or an incorrectly formed -tp
value.

Solution:

Reworked the way that tasks are selected. We now have:

```bash
openjd run template.json --task-param Foo=Bar --task-param Buz=Baz ... # -> Run a single task
openjd run template.json --tasks file://params.json # -> one or more tasks as defined in the file
openjd run template.json --tasks '[{ ... inline json...}]' # -> one or more tasks

```

That is, the `--task-param/-tp` option is now exclusively for the case
where you want to run a single task, and must be provided once for each
task parameter that needs to be defined.

Furthermore, the `--task-param/-tp`, `--tasks`, and `--maximum-tasks`
options have been made mutually exclusive.

Signed-off-by: Daniel Neilson <53624638+ddneilson@users.noreply.github.com>
  • Loading branch information
@ddneilson
ddneilson committed Feb 9, 2024
1 parent 53cbcd9 commit e09d989
Show file tree
Hide file tree
Showing 7 changed files with 482 additions and 258 deletions.
12 changes: 6 additions & 6 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,9 @@ details on how Open Job Description's Jobs are run within Sessions.
|`--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| The values for the job template's parameters. Can be provided as key-value pairs, inline JSON string, or as path(s) 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`|
|`--task-params`, `-tp`|string|no| Instructs the command to run a single task in a Session with the given value for one of the task parameters. The option must be provided once for each task parameter defined for the Step, with each instance providing the value for a different task parameter. Mutually exclusive with `--tasks` and `--maximum-tasks`. |`-tp MyParam=5 -tp MyOtherParam=Foo`|
|`--tasks`|string, path|no| Instructs the command to run one or more tasks for the Step in a Session. The argument must be either the filename of a JSON or YAML file containing an array of maps from task parameter name to value; or an inlined JSON string of the same. Mutually exclusive with `--task-param/-tp` and `--maximum-tasks`. |`--tasks '[{"MyParam": 5}]'`, `--tasks 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 the Task(s) selected by the `--task-params` or `--tasks` arguments. Mutually exclusive with `--task-param/-tp` and `--tasks`. |`--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`|
|`--path-mapping-rules`|string, path|no| The path mapping rules to apply to the template. Should be a JSON-formatted list of Open Job Description path mapping rules, provided as a string or a path to a JSON/YAML document prefixed with 'file://'. |`--path-mapping-rules [{"source_os": "Windows", "source_path": "C:\test", "destination_path": "/mnt/test"}]`, `--path-mapping-rules file://rules_file.json`|
|`--output`|string|no| How to display the results of the command. Allowed values are `human-readable` (default), `json`, and `yaml`. |`--output json`, `--output yaml`|
Expand All @@ -122,9 +123,8 @@ details on how Open Job Description's Jobs are run within Sessions.
```sh
$ openjd-cli run /path/to/job.template.json --step Step1 \
--job-param PingServer=amazon.com \
--task-params PingCount=20 PingDelay=30 \
--task-params file://some_task_parameter_set.json
--maximum-tasks 5
--task-params PingCount=20 \
--task-params PingDelay=30

# ... Task logs accompanied by timestamps ...

Expand All @@ -134,7 +134,7 @@ Session ended successfully
Job: MyJob
Step: Step1
Duration: 1.0 seconds
Tasks run: 5
Tasks run: 1

```

Expand Down
4 changes: 2 additions & 2 deletions src/openjd/cli/_common/__init__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from ._job_from_template import (
job_from_template,
get_job_params,
get_task_params,
get_params_from_file,
)
from ._validation_utils import (
get_doc_type,
Expand All @@ -25,7 +25,7 @@
__all__ = [
"get_doc_type",
"get_job_params",
"get_task_params",
"get_params_from_file",
"read_template",
"read_job_template",
"read_environment_template",
Expand Down
77 changes: 1 addition & 76 deletions src/openjd/cli/_common/_job_from_template.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def get_job_params(parameter_args: list[str]) -> dict:
parameter_dict.update(parameters)

# Case 3: Provided argument is a Key=Value string
elif regex_match := re.match("^(.+)=(.*)$", arg):
elif regex_match := re.match("^([^=]+)=(.*)$", arg):
parameter_dict.update({regex_match[1]: regex_match[2]})

else:
Expand All @@ -102,81 +102,6 @@ def get_job_params(parameter_args: list[str]) -> dict:
return parameter_dict


def get_task_params(arguments: list[list[str]]) -> list[dict[str, str]]:
"""
Retrieves Task parameter sets from user-provided command line arguments.
Each argument may be a list of Task parameters that forms
the parameter set, or a file containing the Task parameter set(s) to use.
For example, the arguments `["Param1=1 Param2=String1", "Param1=2 Param2=String2"]` will produce the following output:
```
[
{
"Param1": "1",
"Param2": "String1"
},
{
"Param1": "2",
"Param2": "String2"
}
]
```
Returns: A list of dictionaries, with each dictionary representing a
Task parameter set. All values are represented as strings regardless
of the parameter's defined type (types are resolved later by the
`sessions` module).
Raises: RuntimeError if filepaths can't be resolved or if arguments
can't be serialized into dictionary objects.
"""
all_parameter_sets: list[dict] = []

error_list: list[str] = []
for arg_list in arguments:
# Case 1: Provided argument is a filepath
if len(arg_list) == 1 and arg_list[0].startswith("file://"):
filename = arg_list[0]
# Raises: RuntimeError
file_parameters = get_params_from_file(filename)
# If the file contains a dictionary, add it as-is
if isinstance(file_parameters, dict):
all_parameter_sets.append(file_parameters)

# If not, the file is a list; check if the list only contains dictionaries,
# with a proper error message if not
elif not all([isinstance(entry, dict) for entry in file_parameters]):
error_list.append(
f"'{filename.removeprefix('file://')}' contains non-dictionary entries: {[entry for entry in file_parameters if not isinstance(entry, dict)]}"
)

# If not, all entries are dictionaries; add them to the parameter sets
else:
all_parameter_sets.extend(file_parameters)

# Case 2: Provided argument is a list of Key=Value strings
else:
parameter_set: dict = {}

for kvp in arg_list:
regex_match = re.match("(.+)=(.+)", kvp.strip())
if not regex_match:
error_list.append(f"'{kvp}' should be in the format 'Key=Value'")
else:
parameter_set.update({regex_match[1]: regex_match[2]})

if parameter_set:
all_parameter_sets.append(parameter_set)

if error_list:
error_msg = "Found the following errors collecting Task parameters:"
for error in error_list:
error_msg += f"\n- {error}"
raise RuntimeError(error_msg)

return all_parameter_sets


def job_from_template(
template: JobTemplate,
parameter_args: list[str] | None,
Expand Down
Loading

0 comments on commit e09d989

Please sign in to comment.