Skip to content

Commit

Permalink
feat(cli): allow disabling automated parameter detection in renku run (
Browse files Browse the repository at this point in the history
  • Loading branch information
Panaetius authored Jul 6, 2023
1 parent 020434a commit bcdeba1
Show file tree
Hide file tree
Showing 6 changed files with 155 additions and 53 deletions.
106 changes: 72 additions & 34 deletions renku/core/workflow/plan_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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.")
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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]
Expand Down
38 changes: 20 additions & 18 deletions renku/core/workflow/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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()},
)
Expand Down
38 changes: 38 additions & 0 deletions renku/domain_model/workflow/parameter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down Expand Up @@ -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."""
Expand Down Expand Up @@ -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."""
Expand Down Expand Up @@ -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."""
Expand Down
13 changes: 13 additions & 0 deletions renku/ui/cli/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -537,6 +547,7 @@ def run(
no_output,
no_input_detection,
no_output_detection,
no_parameter_detection,
success_codes,
isolation,
file,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions tests/api/test_parameter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
11 changes: 10 additions & 1 deletion tests/cli/test_output_option.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit bcdeba1

Please sign in to comment.