Repository: valohai/valohai-cli

Issue:
Add override for pipeline parameters
Currently we cannot override Pipeline parameters through cli

Discussion:
ruksi: > Tests are being failed because it's not using latest valohai-yaml which returns parameters in convertedPipeline object

I guess we'll need to release new YAML version and/or upgrade dependency versions in the CLI before this can be rebased and merged  :+1: 

Patch:
diff --git a/valohai_cli/commands/pipeline/run/run.py b/valohai_cli/commands/pipeline/run/run.py
index b30647f..ca57706 100644
--- a/valohai_cli/commands/pipeline/run/run.py
+++ b/valohai_cli/commands/pipeline/run/run.py
@@ -23,6 +23,7 @@ from valohai_cli.utils.commits import create_or_resolve_commit
 @click.option('--adhoc', '-a', is_flag=True, help='Upload the current state of the working directory, then run it as an ad-hoc execution.')
 @click.option('--yaml', default=None, help='The path to the configuration YAML file (valohai.yaml) file to use.')
 @click.option('--tag', 'tags', multiple=True, help='Tag the pipeline. May be repeated.')
+@click.argument('args', nargs=-1, type=click.UNPROCESSED, metavar='PIPELINE-OPTIONS...')
 @click.pass_context
 def run(
     ctx: Context,
@@ -32,6 +33,7 @@ def run(
     adhoc: bool,
     yaml: Optional[str],
     tags: List[str],
+    args: List[str],
 ) -> None:
     """
     Start a pipeline run.
@@ -55,7 +57,42 @@ def run(
     matched_pipeline = match_pipeline(config, name)
     pipeline = config.pipelines[matched_pipeline]
 
-    start_pipeline(config, pipeline, project.id, commit, tags, title)
+    start_pipeline(config, pipeline, project.id, commit, tags, args, title)
+
+
+def override_pipeline_parameters(args: List[str], pipeline_parameters: Dict[str, Any]) -> Dict[str, Any]:
+    args_dict = process_args(args)
+    if not pipeline_parameters and args_dict:
+        raise click.UsageError('Pipeline does not have any parameters')
+
+    for param in pipeline_parameters:
+        if param in args_dict:
+            pipeline_parameters[param]['expression'] = args_dict[param]
+            args_dict.pop(param)
+
+    if args_dict:
+        raise click.UsageError(f'Unknown pipeline parameter {list(args_dict.keys())}')
+
+    return pipeline_parameters
+
+
+def process_args(args: List[str]) -> Dict[str, str]:
+    i = 0
+    args_dict = {}
+    for arg in args:
+        if arg.startswith("--"):
+            arg_name = arg.lstrip("-")
+            if "=" in arg_name: # --param=value
+                name, value = arg_name.split("=", 1)
+                args_dict[name] = value
+            else: # --param value
+                next_arg_idx = i + 1
+                if next_arg_idx < len(args) and not args[next_arg_idx].startswith("--"):
+                    args_dict[arg_name] = args[next_arg_idx]
+                else: # --param --param2 --param3 (flag)
+                    args_dict[arg_name] = "true" # doesn't support bool as we are using strings for pipeline parameters
+        i += 1
+    return args_dict
 
 
 def print_pipeline_list(ctx: Context, commit: Optional[str]) -> None:
@@ -70,18 +107,22 @@ def print_pipeline_list(ctx: Context, commit: Optional[str]) -> None:
 
 
 def start_pipeline(
-    config: Config,
-    pipeline: Pipeline,
-    project_id: str,
-    commit: str,
-    tags: List[str],
-    title: Optional[str] = None,
+        config: Config,
+        pipeline: Pipeline,
+        project_id: str,
+        commit: str,
+        tags: List[str],
+        args: List[str],
+        title: Optional[str] = None,
 ) -> None:
+    converted_pipeline = PipelineConverter(config=config, commit_identifier=commit).convert_pipeline(pipeline)
+    if args:
+        converted_pipeline['parameters'] = override_pipeline_parameters(args, converted_pipeline['parameters'])
     payload: Dict[str, Any] = {
         "project": project_id,
         "title": title or pipeline.name,
         "tags": tags,
-        **PipelineConverter(config=config, commit_identifier=commit).convert_pipeline(pipeline),
+        **converted_pipeline,
     }
 
     resp = request(


Test patch:
diff --git a/tests/commands/pipeline/test_run.py b/tests/commands/pipeline/test_run.py
index b4d589b..0481d84 100644
--- a/tests/commands/pipeline/test_run.py
+++ b/tests/commands/pipeline/test_run.py
@@ -67,3 +67,27 @@ def add_valid_pipeline_yaml(yaml_path=None):
     config_filename = project.get_config_filename(yaml_path=yaml_path)
     with open(config_filename, 'w') as yaml_fp:
         yaml_fp.write(PIPELINE_YAML)
+
+
+def test_pipeline_parameters_overriding(runner, logged_in_and_linked):
+    add_valid_pipeline_yaml()
+    # Test if it lets unknown pipeline parameters pass through
+    overriding_value = '123'
+    args = ['--adhoc', 'Train Pipeline', '--not-known',  overriding_value]
+    with RunAPIMock(PROJECT_DATA['id']):
+        output = runner.invoke(run, args).output
+        assert "Unknown pipeline parameter ['not-known']" in output
+
+    args = ['--adhoc', 'Train Pipeline', '--pipeline_max_steps', overriding_value]
+    with RunAPIMock(PROJECT_DATA['id']) as mock_api:
+        output = runner.invoke(run, args).output
+
+        # Test that it runs successfully
+        assert 'Success' in output
+        assert 'Uploaded ad-hoc code' in output
+        assert 'Pipeline =21 queued' in output
+
+        # Test that the pipeline parameter was overridden
+
+        payload = mock_api.last_create_pipeline_payload['parameters']['pipeline_max_steps']
+        assert payload['expression'] == overriding_value
diff --git a/tests/commands/run_test_utils.py b/tests/commands/run_test_utils.py
index ed8659c..75900df 100644
--- a/tests/commands/run_test_utils.py
+++ b/tests/commands/run_test_utils.py
@@ -34,6 +34,7 @@ class RunAPIMock(requests_mock.Mocker):
     ):
         super().__init__()
         self.last_create_execution_payload = None
+        self.last_create_pipeline_payload = None
         self.project_id = project_id
         self.commit_id = commit_id
         self.deployment_id = deployment_id
@@ -152,7 +153,10 @@ class RunAPIMock(requests_mock.Mocker):
         assert body_json['project'] == self.project_id
         assert len(body_json['edges']) == 5
         assert len(body_json['nodes']) == 3
+        if "parameters" in body_json:
+            assert len(body_json['parameters']) == 1
         context.status_code = 201
+        self.last_create_pipeline_payload = body_json
         return PIPELINE_DATA.copy()
 
     def handle_create_commit(self, request, context):
diff --git a/tests/fixture_data.py b/tests/fixture_data.py
index eb964b8..d28a05b 100644
--- a/tests/fixture_data.py
+++ b/tests/fixture_data.py
@@ -360,6 +360,11 @@ PIPELINE_YAML = """
       - [preprocess.output.*test-images*, train.input.test-set-images]
       - [preprocess.output.*test-labels*, train.input.test-set-labels]
       - [train.output.model*, evaluate.input.model]
+    parameters:
+      - name: pipeline_max_steps
+        default: 1000
+        targets:
+            - Train model (MNIST).parameters.max_steps
 
 - pipeline:
     name: Train Pipeline
@@ -385,6 +390,11 @@ PIPELINE_YAML = """
       - [preprocess.output.*test-images*, train.input.test-set-images]
       - [preprocess.output.*test-labels*, train.input.test-set-labels]
       - [train.output.model*, evaluate.input.model]
+    parameters:
+      - name: pipeline_max_steps
+        default: 1000
+        targets:
+            - Train model (MNIST).parameters.max_steps
 """
 
 YAML_WITH_EXTRACT_TRAIN_EVAL = """