feat(cli): allow disabling automated parameter detection in renku run (#3548)

Author: Panaetius

renku/core/workflow/plan_factory.py

@@ -71,6 +71,7 @@ def __init__(
         working_dir: Optional[Union[Path, str]] = None,
         no_input_detection: bool = False,
         no_output_detection: bool = False,
+        no_parameter_detection: bool = False,
         success_codes: Optional[List[int]] = None,
         stdin: Optional[str] = None,
         stdout: Optional[str] = None,
@@ -80,6 +81,7 @@ def __init__(
 
         self.no_input_detection = no_input_detection
         self.no_output_detection = no_output_detection
+        self.no_parameter_detection = no_parameter_detection
 
         if not command_line:
             raise errors.UsageError("Command line can not be empty. Please specify a command to execute.")
@@ -392,23 +394,34 @@ def add_command_input(
 
         mapped_stream = self.get_stream_mapping_for_value(default_value)
 
-        self.inputs.append(
-            CommandInput(
-                id=CommandInput.generate_id(
-                    plan_id=self.plan_id,
-                    position=position,
-                    postfix=mapped_stream.stream_type if mapped_stream else postfix,
-                ),
-                default_value=default_value,
-                prefix=prefix,
+        inp_param = CommandInput(
+            id=CommandInput.generate_id(
+                plan_id=self.plan_id,
                 position=position,
-                mapped_to=mapped_stream,
-                encoding_format=encoding_format,
-                postfix=postfix,
-                name=name,
-            )
+                postfix=mapped_stream.stream_type if mapped_stream else postfix,
+            ),
+            default_value=default_value,
+            prefix=prefix,
+            position=position,
+            mapped_to=mapped_stream,
+            encoding_format=encoding_format,
+            postfix=postfix,
+            name=name,
         )
 
+        existing_parameter = next((p for p in self.inputs if p.name == inp_param.name), None)
+
+        if existing_parameter is not None and existing_parameter.default_value == inp_param.default_value:
+            existing_parameter.update_from(inp_param)
+        elif existing_parameter is not None:
+            # duplicate with different values!
+            raise errors.ParameterError(
+                f"Duplicate input '{inp_param.name}' found with differing values ('{inp_param.default_value}'"
+                f" vs. '{existing_parameter.default_value}')"
+            )
+        else:
+            self.inputs.append(inp_param)
+
     def add_command_output(
         self,
         default_value: Any,
@@ -447,20 +460,31 @@ def add_command_output(
             postfix=mapped_stream.stream_type if mapped_stream else postfix,
         )
 
-        self.outputs.append(
-            CommandOutput(
-                id=id,
-                default_value=default_value,
-                prefix=prefix,
-                position=position,
-                mapped_to=mapped_stream,
-                encoding_format=encoding_format,
-                postfix=postfix,
-                create_folder=create_folder,
-                name=name,
-            )
+        out_param = CommandOutput(
+            id=id,
+            default_value=default_value,
+            prefix=prefix,
+            position=position,
+            mapped_to=mapped_stream,
+            encoding_format=encoding_format,
+            postfix=postfix,
+            create_folder=create_folder,
+            name=name,
         )
 
+        existing_parameter = next((p for p in self.outputs if p.name == out_param.name), None)
+
+        if existing_parameter is not None and existing_parameter.default_value == out_param.default_value:
+            existing_parameter.update_from(out_param)
+        elif existing_parameter is not None:
+            # duplicate with different values!
+            raise errors.ParameterError(
+                f"Duplicate output '{out_param.name}' found with differing values ('{out_param.default_value}'"
+                f" vs. '{existing_parameter.default_value}')"
+            )
+        else:
+            self.outputs.append(out_param)
+
     def add_command_output_from_input(self, input: CommandInput, name):
         """Create a CommandOutput from an input."""
         self.inputs.remove(input)
@@ -496,16 +520,30 @@ def add_command_parameter(
         name: Optional[str] = None,
     ):
         """Create a CommandParameter."""
-        self.parameters.append(
-            CommandParameter(
-                id=CommandParameter.generate_id(plan_id=self.plan_id, position=position),
-                default_value=default_value,
-                prefix=prefix,
-                position=position,
-                name=name,
-            )
+        if self.no_parameter_detection and all(default_value != v for v, _ in self.explicit_parameters):
+            return
+
+        parameter = CommandParameter(
+            id=CommandParameter.generate_id(plan_id=self.plan_id, position=position),
+            default_value=default_value,
+            prefix=prefix,
+            position=position,
+            name=name,
         )
 
+        existing_parameter = next((p for p in self.parameters if p.name == parameter.name), None)
+
+        if existing_parameter is not None and existing_parameter.default_value == parameter.default_value:
+            existing_parameter.update_from(parameter)
+        elif existing_parameter is not None:
+            # duplicate with different values!
+            raise errors.ParameterError(
+                f"Duplicate parameter '{parameter.name}' found with differing values ('{parameter.default_value}'"
+                f" vs. '{existing_parameter.default_value}')"
+            )
+        else:
+            self.parameters.append(parameter)
+
     def add_explicit_inputs(self):
         """Add explicit inputs ."""
         input_paths = [input.default_value for input in self.inputs]

renku/core/workflow/run.py

@@ -167,18 +167,19 @@ def get_valid_parameter_name(name: str) -> str:
 @inject.autoparams("activity_gateway", "plan_gateway")
 @validate_arguments(config=dict(arbitrary_types_allowed=True))
 def run_command_line(
-    name,
-    description,
-    keyword,
-    explicit_inputs,
-    explicit_outputs,
-    explicit_parameters,
-    no_output,
-    no_input_detection,
-    no_output_detection,
-    success_codes,
-    command_line,
-    creators,
+    name: Optional[str],
+    description: Optional[str],
+    keyword: Optional[List[str]],
+    explicit_inputs: List[str],
+    explicit_outputs: List[str],
+    explicit_parameters: List[str],
+    no_output: bool,
+    no_input_detection: bool,
+    no_output_detection: bool,
+    no_parameter_detection: bool,
+    success_codes: List[int],
+    command_line: List[str],
+    creators: Optional[List[Person]],
     activity_gateway: IActivityGateway,
     plan_gateway: IPlanGateway,
 ) -> PlanViewModel:
@@ -262,19 +263,20 @@ def parse_explicit_definition(entries, type):
 
             return result
 
-        explicit_inputs = parse_explicit_definition(explicit_inputs, "input")
-        explicit_outputs = parse_explicit_definition(explicit_outputs, "output")
-        explicit_parameters = parse_explicit_definition(explicit_parameters, "param")
+        explicit_inputs_parsed = parse_explicit_definition(explicit_inputs, "input")
+        explicit_outputs_parsed = parse_explicit_definition(explicit_outputs, "output")
+        explicit_parameters_parsed = parse_explicit_definition(explicit_parameters, "param")
 
         factory = PlanFactory(
             command_line=command_line,
-            explicit_inputs=explicit_inputs,
-            explicit_outputs=explicit_outputs,
-            explicit_parameters=explicit_parameters,
+            explicit_inputs=explicit_inputs_parsed,
+            explicit_outputs=explicit_outputs_parsed,
+            explicit_parameters=explicit_parameters_parsed,
             directory=os.getcwd(),
             working_dir=working_dir,
             no_input_detection=no_input_detection,
             no_output_detection=no_output_detection,
+            no_parameter_detection=no_parameter_detection,
             success_codes=success_codes,
             **{name: os.path.relpath(path, working_dir) for name, path in mapped_std.items()},
         )

renku/domain_model/workflow/parameter.py

@@ -159,6 +159,17 @@ def to_argv(self, quote_string: bool = True) -> List[Any]:
 
         return [str(value)]
 
+    def _update_from(self, other: "CommandParameterBase"):
+        """Update this parameter with values from another parameter, if applicable."""
+        if other.prefix is not None:
+            self.prefix = other.prefix
+        if other.position is not None:
+            self.position = other.position
+        if other.description:
+            self.description = other.description
+        if other.name_set_by_user:
+            self.name_set_by_user = other.name_set_by_user
+
     @property
     def actual_value(self):
         """Get the actual value to be used for execution."""
@@ -242,6 +253,10 @@ def derive(self, plan_id: str) -> "CommandParameter":
         parameter.id = CommandParameter.generate_id(plan_id=plan_id, position=self.position, postfix=self.postfix)
         return parameter
 
+    def update_from(self, other: "CommandParameter"):
+        """Update this output with values from another output, if applicable."""
+        super()._update_from(other)
+
 
 class CommandInput(CommandParameterBase):
     """An input to a command."""
@@ -317,6 +332,16 @@ def derive(self, plan_id: str) -> "CommandInput":
         parameter.id = CommandInput.generate_id(plan_id=plan_id, position=self.position, postfix=self.postfix)
         return parameter
 
+    def update_from(self, other: "CommandInput"):
+        """Update this input with values from another input, if applicable."""
+        super()._update_from(other)
+
+        if other.encoding_format:
+            self.encoding_format = other.encoding_format
+
+        if other.mapped_to is not None:
+            self.mapped_to = other.mapped_to
+
 
 class HiddenInput(CommandInput):
     """An input to a command that is added by Renku and should be hidden from users."""
@@ -391,6 +416,19 @@ def is_equal_to(self, other) -> bool:
 
         return super().is_equal_to(other)
 
+    def update_from(self, other: "CommandOutput"):
+        """Update this output with values from another output, if applicable."""
+        super()._update_from(other)
+
+        if other.encoding_format:
+            self.encoding_format = other.encoding_format
+
+        if other.mapped_to is not None:
+            self.mapped_to = other.mapped_to
+
+        if other.create_folder:
+            self.create_folder = other.create_folder
+
     @staticmethod
     def _get_equality_attributes() -> List[str]:
         """Return a list of attributes values that determine if instances are equal."""

renku/ui/cli/run.py

@@ -132,6 +132,15 @@
     This only affects files and directories; command options and flags are
     still treated as inputs.
 
+.. topic:: Disabling parameter detection (``--no-parameter-detection``)
+
+    Inputs that aren't files or directories are automatically detected as
+    parameters in ``renku run``. You can disable this feature by passing the
+    ``--no-parameter-detection`` flag, which completely ignores them on the
+    workflow. You can still manually specify parameters using ``--param``
+    arguments mentioned above or using the ``renku.api.Parameter`` class in
+    Python code.
+
 .. note:: ``renku run`` prints the generated plan after execution if you pass
     ``--verbose`` to it. You can check the generated plan to verify that the
     execution was done as you intended. The plan will always be printed to
@@ -498,6 +507,7 @@
 @click.option("--no-output", is_flag=True, default=False, help="Allow command without output files.")
 @click.option("--no-input-detection", is_flag=True, default=False, help="Disable auto-detection of inputs.")
 @click.option("--no-output-detection", is_flag=True, default=False, help="Disable auto-detection of outputs.")
+@click.option("--no-parameter-detection", is_flag=True, default=False, help="Disable auto-detection of parameters.")
 @click.option(
     "--success-code",
     "success_codes",
@@ -537,6 +547,7 @@ def run(
     no_output,
     no_input_detection,
     no_output_detection,
+    no_parameter_detection,
     success_codes,
     isolation,
     file,
@@ -587,6 +598,7 @@ def is_workflow_file() -> bool:
             or no_output
             or no_input_detection
             or no_output_detection
+            or no_parameter_detection
             or success_codes
             or isolation
             or creators
@@ -653,6 +665,7 @@ def is_workflow_file() -> bool:
                 no_output=no_output,
                 no_input_detection=no_input_detection,
                 no_output_detection=no_output_detection,
+                no_parameter_detection=no_parameter_detection,
                 success_codes=success_codes,
                 command_line=command_line,
                 creators=creators,

tests/api/test_parameter.py

@@ -121,6 +121,8 @@ def test_parameters(project):
 
     assert (42, "42", 42.42) == (p1.value, p2.value, p3.value)
 
+    _ = Parameter("parameter_3", 42.42)
+
     data = read_indirect_parameters(project.path)
 
     assert {"parameter-1", "param-2", "parameter_3"} == set(data.keys())

tests/cli/test_output_option.py

@@ -279,12 +279,21 @@ def test_no_output_and_disabled_detection(renku_cli):
 def test_disabled_detection(renku_cli):
     """Test disabled auto-detection of inputs and outputs."""
     exit_code, activity = renku_cli(
-        "run", "--no-input-detection", "--no-output-detection", "--output", "README.md", "touch", "some-files"
+        "run",
+        "--no-input-detection",
+        "--no-output-detection",
+        "--no-parameter-detection",
+        "--output",
+        "README.md",
+        "touch",
+        "some-files",
+        "-f",
     )
 
     assert 0 == exit_code
     plan = activity.association.plan
     assert 0 == len(plan.inputs)
+    assert 0 == len(plan.parameters)
     assert 1 == len(plan.outputs)
     assert "README.md" == str(plan.outputs[0].default_value)