From 27800b4e3353762147baae502780c7597164d3dd Mon Sep 17 00:00:00 2001 From: John Chilton Date: Tue, 15 Oct 2019 09:51:15 -0400 Subject: [PATCH] Richer workflow inputs. - Allow parameter inputs to be optional. - Allow parameter inputs to have default values (if optional). - Allow data and collection parameter to be optional. - Unimplemented but new abstractions should allow having users specify extensions manually if they'd like for data and collection parameters. The previous workflow Module interface would more or less persist Galaxy's "Tool State" directly into the database. The tool state framework persists state in an un-typed fashion (e.g. the optional value for data inputs would be 'True' instead of True). The Format 2 YAML that we would like to publish to Galaxy and export from Galaxy would much more cleanly represent this as actual boolean values. The problem gets even worse with parameter inputs where we want the default value to be settable only if optional is True. This requires a conditional in the "Tool Form" language - so the persisted value would be nested structures without typing. For instance - instead of ``{default: 5, parameter_type: "integer", "optional": true}`` that we might want to store into the database or read from a workflow definition file, the framework would require something like ``{"parameter_type": "integer", "parameter_defintion": {"optional": {"optional": "True", {"specify_optional": {"specify_default": "True", "default": "5"}}}}}``. My preference may have been to have plugins defined in the client that just produce the minimal JSON - but I didn't know how to implement that and Sam's preference was that the client use the tool form framework for everything on the side panel anyway though. As a result, this commit adds abstractions to separate module tool state (for commiunicating with client and tied to tool form rendering) from abstract export state - loaded from the API when not coming from the tool and stored in database. To summarize workflow module changes... - module.get_inputs can build complex tool form parameteras (unchanged) - module.state is always the tool form state (unchanged) - module.get_export_state is added to get the abstract state for export and for reasoning about internally - module.recover_state now assumes recovery from abstract step state until from_tool_form=True is supplied - API entry points tied to the tool form (build_module) just pass through from_tool_form by default. - The client now sends from_tool_form into the API whenever it serializes the workflow state for re-saving --- .../scripts/mvc/tool/tool-form-composite.js | 3 + .../scripts/mvc/workflow/workflow-view.js | 3 +- lib/galaxy/managers/workflows.py | 16 +- lib/galaxy/model/__init__.py | 11 + lib/galaxy/tools/parameters/__init__.py | 7 +- lib/galaxy/webapps/base/controller.py | 3 +- lib/galaxy/webapps/galaxy/api/workflows.py | 8 +- lib/galaxy/workflow/modules.py | 282 ++++++++++++++++-- lib/galaxy/workflow/run.py | 10 +- lib/galaxy/workflow/run_request.py | 13 +- lib/galaxy_test/api/test_workflows.py | 118 +++++++- lib/galaxy_test/base/populators.py | 3 + lib/galaxy_test/base/workflow_fixtures.py | 138 +++++++++ 13 files changed, 567 insertions(+), 48 deletions(-) diff --git a/client/galaxy/scripts/mvc/tool/tool-form-composite.js b/client/galaxy/scripts/mvc/tool/tool-form-composite.js index fe1512fee99..f1f97d78a79 100644 --- a/client/galaxy/scripts/mvc/tool/tool-form-composite.js +++ b/client/galaxy/scripts/mvc/tool/tool-form-composite.js @@ -598,6 +598,9 @@ var View = Backbone.View.extend({ if (!input_def.step_linked) { if (this._isDataStep(step)) { validated = input_value && input_value.values && input_value.values.length > 0; + if (!validated && input_def.optional) { + validated = true; + } } else { validated = input_def.optional || diff --git a/client/galaxy/scripts/mvc/workflow/workflow-view.js b/client/galaxy/scripts/mvc/workflow/workflow-view.js index 3538ba432aa..64430062548 100644 --- a/client/galaxy/scripts/mvc/workflow/workflow-view.js +++ b/client/galaxy/scripts/mvc/workflow/workflow-view.js @@ -689,7 +689,7 @@ export default Backbone.View.extend({ Utils.request({ url: `${getAppRoot()}api/workflows/${self.options.id}`, type: "PUT", - data: { workflow: self.workflow.to_simple() }, + data: { workflow: self.workflow.to_simple(), from_tool_form: true }, success: function(data) { var body = $("
").text(data.message); if (data.errors) { @@ -743,6 +743,7 @@ export default Backbone.View.extend({ data: { workflow_name: rename_name, workflow_annotation: rename_annotation, + from_tool_form: true, workflow_data: function() { return JSON.stringify(self.workflow.to_simple()); } diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 3d8862eb18a..5980cd8e0e2 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -300,6 +300,7 @@ class WorkflowContentsManager(UsesAnnotations): create_stored_workflow=True, exact_tools=True, fill_defaults=False, + from_tool_form=False, ): data = raw_workflow_description.as_dict # Put parameters in workflow mode @@ -315,6 +316,7 @@ class WorkflowContentsManager(UsesAnnotations): name=name, exact_tools=exact_tools, fill_defaults=fill_defaults, + from_tool_form=from_tool_form, ) if 'uuid' in data: workflow.uuid = data['uuid'] @@ -624,7 +626,7 @@ class WorkflowContentsManager(UsesAnnotations): 'label': module.label, 'content_id': module.get_content_id(), 'name': module.get_name(), - 'tool_state': module.get_state(), + 'tool_state': module.get_tool_state(), 'errors': module.get_errors(), 'inputs': module.get_all_inputs(connectable_only=True), 'outputs': module.get_all_outputs(), @@ -823,10 +825,7 @@ class WorkflowContentsManager(UsesAnnotations): annotation_str = self.get_item_annotation_str(trans.sa_session, trans.user, step) or '' content_id = module.get_content_id() # Export differences for backward compatibility - if module.type == 'tool': - tool_state = module.get_state(nested=False) - else: - tool_state = module.state.inputs + tool_state = module.get_export_state() # Step info step_dict = { 'id': step.order_index, @@ -885,9 +884,10 @@ class WorkflowContentsManager(UsesAnnotations): # Data inputs, legacy section not used anywhere within core input_dicts = [] step_state = module.state.inputs or {} - if "name" in step_state and module.type != 'tool': - name = step_state.get("name") - input_dicts.append({"name": name, "description": annotation_str}) + if module.type != 'tool': + name = step_state.get("name") or module.label + if name: + input_dicts.append({"name": name, "description": annotation_str}) for name, val in step_state.items(): input_type = type(val) if input_type == RuntimeValue: diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 1f1f3f2eb70..d9e61dbd973 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -4658,6 +4658,15 @@ class WorkflowStep(RepresentById): def tool_uuid(self): return self.dynamic_tool and self.dynamic_tool.uuid + @property + def input_default_value(self): + tool_inputs = self.tool_inputs + tool_state = tool_inputs + default_value = tool_state.get("default") + if default_value: + default_value = json.loads(default_value)["value"] + return default_value + def get_input(self, input_name): for step_input in self.inputs: if step_input.name == input_name: @@ -5115,6 +5124,8 @@ class WorkflowInvocation(UsesCreateAndUpdateTime, Dictifiable, RepresentById): if output_step_type in ['data_input', 'data_collection_input']: src = "hda" if output_step_type == 'data_input' else 'hdca' for job_input in job.input_datasets: + if not job_input.dataset: + continue if job_input.name == step_input.input_name: inputs[str(step_input.output_step.order_index)] = { "id": job_input.dataset_id, "src": src, diff --git a/lib/galaxy/tools/parameters/__init__.py b/lib/galaxy/tools/parameters/__init__.py index 206b62311af..4ecccc583cd 100644 --- a/lib/galaxy/tools/parameters/__init__.py +++ b/lib/galaxy/tools/parameters/__init__.py @@ -19,7 +19,7 @@ REPLACE_ON_TRUTHY = object() __all__ = ('DataCollectionToolParameter', 'DataToolParameter', 'SelectToolParameter') -def visit_input_values(inputs, input_values, callback, name_prefix='', label_prefix='', parent_prefix='', context=None, no_replacement_value=REPLACE_ON_TRUTHY): +def visit_input_values(inputs, input_values, callback, name_prefix='', label_prefix='', parent_prefix='', context=None, no_replacement_value=REPLACE_ON_TRUTHY, replace_optional_connections=False): """ Given a tools parameter definition (`inputs`) and a specific set of parameter `values`, call `callback` for each non-grouping parameter, @@ -116,10 +116,11 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre No value found for 'b 1 > d 1 > j'. """ def callback_helper(input, input_values, name_prefix, label_prefix, parent_prefix, context=None, error=None): + value = input_values.get(input.name) args = { 'input' : input, 'parent' : input_values, - 'value' : input_values.get(input.name), + 'value' : value, 'prefixed_name' : '%s%s' % (name_prefix, input.name), 'prefixed_label' : '%s%s' % (label_prefix, input.label or input.name), 'prefix' : parent_prefix, @@ -135,6 +136,8 @@ def visit_input_values(inputs, input_values, callback, name_prefix='', label_pre replace = new_value != no_replacement_value if replace: input_values[input.name] = new_value + elif replace_optional_connections and is_runtime_value(value): + input_values[input.name] = input.value def get_current_case(input, input_values): try: diff --git a/lib/galaxy/webapps/base/controller.py b/lib/galaxy/webapps/base/controller.py index f08c3b81d0b..b0d7e8b5b3f 100644 --- a/lib/galaxy/webapps/base/controller.py +++ b/lib/galaxy/webapps/base/controller.py @@ -1249,7 +1249,7 @@ class UsesStoredWorkflowMixin(SharableItemSecurityMixin, UsesAnnotations): session.flush() return imported_stored - def _workflow_from_dict(self, trans, data, source=None, add_to_menu=False, publish=False, exact_tools=True, fill_defaults=False): + def _workflow_from_dict(self, trans, data, source=None, add_to_menu=False, publish=False, exact_tools=True, fill_defaults=False, from_tool_form=False): """ Creates a workflow from a dict. Created workflow is stored in the database and returned. """ @@ -1264,6 +1264,7 @@ class UsesStoredWorkflowMixin(SharableItemSecurityMixin, UsesAnnotations): publish=publish, exact_tools=exact_tools, fill_defaults=fill_defaults, + from_tool_form=from_tool_form, ) return created_workflow.stored_workflow, created_workflow.missing_tools diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 79aed89c023..68f47a11942 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -551,6 +551,7 @@ class WorkflowsAPIController(BaseAPIController, UsesStoredWorkflowMixin, UsesAnn annotation defaults to existing annotation * menu_entry optional boolean marking if the workflow should appear in the user's menu, if not present, workflow menu entries are not modified + * from_tool_form True iff encoded state coming in is encoded for the tool form. :rtype: dict :returns: serialized version of the workflow @@ -613,11 +614,11 @@ class WorkflowsAPIController(BaseAPIController, UsesStoredWorkflowMixin, UsesAnn """ inputs = payload.get('inputs', {}) trans.workflow_building_mode = workflow_building_modes.ENABLED - module = module_factory.from_dict(trans, payload) + module = module_factory.from_dict(trans, payload, from_tool_form=True) if 'tool_state' not in payload: module_state = {} populate_state(trans, module.get_inputs(), inputs, module_state, check=False) - module.recover_state(module_state) + module.recover_state(module_state, from_tool_form=True) return { 'label' : inputs.get('__label', ''), 'annotation' : inputs.get('__annotation', ''), @@ -719,10 +720,11 @@ class WorkflowsAPIController(BaseAPIController, UsesStoredWorkflowMixin, UsesAnn # Fill in missing tool state for hand built so the workflow can run, default of this # should become True at some point in the future I imagine. fill_defaults = util.string_as_bool(payload.get("fill_defaults", False)) - + from_tool_form = payload.get("from_tool_form", False) return { 'exact_tools': exact_tools, 'fill_defaults': fill_defaults, + 'from_tool_form': from_tool_form, } def __normalize_workflow(self, trans, as_dict): diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 57c6f2102e1..83fd3c20589 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -38,7 +38,9 @@ from galaxy.tools.parameters.basic import ( ConnectedValue, DataCollectionToolParameter, DataToolParameter, + FloatToolParameter, HiddenToolParameter, + IntegerToolParameter, is_runtime_value, parameter_types, runtime_to_json, @@ -46,6 +48,10 @@ from galaxy.tools.parameters.basic import ( TextToolParameter, workflow_building_modes ) +from galaxy.tools.parameters.grouping import ( + Conditional, + ConditionalWhen, +) from galaxy.tools.parameters.history_query import HistoryQuery from galaxy.tools.parameters.wrapped import make_dict_copy from galaxy.util import unicodify @@ -94,7 +100,7 @@ class WorkflowModule(object): @classmethod def from_workflow_step(Class, trans, step, **kwds): module = Class(trans, **kwds) - module.recover_state(step.tool_inputs) + module.recover_state(step.tool_inputs, from_tool_form=False) module.label = step.label return module @@ -136,13 +142,31 @@ class WorkflowModule(object): else: return self.state.inputs + def get_export_state(self): + return self.get_state(nested=True) + + def get_tool_state(self): + return self.get_state(nested=False) + def recover_state(self, state, **kwds): - """ Recover state `dict` from simple dictionary describing configuration - state (potentially from persisted step state). + """Recover tool state (as self.state and self.state.inputs) from dictionary describing + configuration state (potentially from persisted step state). Sub-classes should supply a `default_state` method which contains the initial state `dict` with key, value pairs for all available attributes. + + Input parameter state may be a typed dictionary or a tool state dictionary generated by + the web client (if from_tool_form in kwds is True). + + If classes need to distinguish between typed clean dictionary representations + of state and the state generated for tool form (with extra nesting logic in the + state for conditionals, un-typed string parameters, etc...) they should + override the step_state_to_tool_state method to make this transformation. """ + from_tool_form = kwds.get("from_tool_form", False) + if not from_tool_form: + state = self.step_state_to_tool_state(state) + self.state = DefaultToolState() inputs = self.get_inputs() if inputs: @@ -150,6 +174,9 @@ class WorkflowModule(object): else: self.state.inputs = safe_loads(state) or {} + def step_state_to_tool_state(self, state): + return state + def get_errors(self, **kwargs): """ This returns a step related error message as string or None """ return None @@ -554,7 +581,13 @@ class InputProxy(object): return as_dict +def optional_param(optional): + optional_value = BooleanToolParameter(None, Element("param", name="optional", label="Optional", type="boolean", checked=optional)) + return optional_value + + class InputModule(WorkflowModule): + default_optional = False def get_runtime_state(self): state = DefaultToolState() @@ -596,6 +629,26 @@ class InputModule(WorkflowModule): def recover_mapping(self, invocation_step, progress): progress.set_outputs_for_input(invocation_step, already_persisted=True) + def get_export_state(self): + return self._parse_state_into_dict() + + def _parse_state_into_dict(self): + inputs = self.state.inputs + rval = {} + if "optional" in inputs: + optional = bool(inputs["optional"]) + else: + optional = self.default_optional + rval["optional"] = optional + return rval + + def step_state_to_tool_state(self, state): + return state + + def save_to_step(self, step): + step.type = self.type + step.tool_inputs = self._parse_state_into_dict() + class InputDataModule(InputModule): type = "data_input" @@ -616,7 +669,19 @@ class InputDataModule(InputModule): return ', '.join(filter_set) def get_runtime_inputs(self, connections=None): - return dict(input=DataToolParameter(None, Element("param", name="input", label=self.label, multiple=False, type="data", format=self.get_filter_set(connections)), self.trans)) + parameter_def = self._parse_state_into_dict() + optional = parameter_def["optional"] + # TODO: extension from parameter_def + input_format = self.get_filter_set(connections) + input_param = DataToolParameter(None, Element("param", name="input", label=self.label, multiple=False, type="data", format=input_format, optional=str(optional)), self.trans) + return dict(input=input_param) + + def get_inputs(self): + parameter_def = self._parse_state_into_dict() + optional = parameter_def["optional"] + inputs = OrderedDict() + inputs["optional"] = optional_param(optional) + return inputs class InputDataCollectionModule(InputModule): @@ -626,7 +691,10 @@ class InputDataCollectionModule(InputModule): collection_type = default_collection_type def get_inputs(self): - collection_type = self.state.inputs.get("collection_type", self.default_collection_type) + parameter_def = self._parse_state_into_dict() + collection_type = parameter_def["collection_type"] + optional = parameter_def["optional"] + # TODO: extensions... input_collection_type = TextToolParameter(None, XML( ''' @@ -635,12 +703,19 @@ class InputDataCollectionModule(InputModule): ''' % collection_type)) - return dict(collection_type=input_collection_type) + + inputs = OrderedDict() + inputs["collection_type"] = input_collection_type + inputs["optional"] = optional_param(optional) + return inputs def get_runtime_inputs(self, **kwds): - collection_type = self.state.inputs.get("collection_type", self.default_collection_type) - input_element = Element("param", name="input", label=self.label, type="data_collection", collection_type=collection_type) - return dict(input=DataCollectionToolParameter(None, input_element, self.trans)) + parameter_def = self._parse_state_into_dict() + collection_type = parameter_def["collection_type"] + optional = parameter_def["optional"] + input_element = Element("param", name="input", label=self.label, type="data_collection", collection_type=collection_type, optional=str(optional)) + input_param = DataCollectionToolParameter(None, input_element, self.trans) + return dict(input=input_param) def get_all_outputs(self, data_only=False): return [ @@ -652,50 +727,151 @@ class InputDataCollectionModule(InputModule): ) ] + def _parse_state_into_dict(self): + state_as_dict = super(InputDataCollectionModule, self)._parse_state_into_dict() + inputs = self.state.inputs + if "collection_type" in inputs: + collection_type = inputs["collection_type"] + else: + collection_type = self.default_collection_type + state_as_dict["collection_type"] = collection_type + return state_as_dict + class InputParameterModule(WorkflowModule): type = "parameter_input" name = "Input parameter" default_parameter_type = "text" default_optional = False + default_default_value = '' parameter_type = default_parameter_type optional = default_optional + default_value = default_default_value def get_inputs(self): - # TODO: Use an external xml or yaml file to load the parameter definition - parameter_type = self.state.inputs.get("parameter_type", self.default_parameter_type) - optional = self.state.inputs.get("optional", self.default_optional) + parameter_def = self._parse_state_into_dict() + parameter_type = parameter_def["parameter_type"] + optional = parameter_def["optional"] input_parameter_type = SelectToolParameter(None, XML( ''' - + - ''')) + ''' % parameter_type)) for i, option in enumerate(input_parameter_type.static_options): option = list(option) if option[1] == parameter_type: # item 0 is option description, item 1 is value, item 2 is "selected" option[2] = True input_parameter_type.static_options[i] = tuple(option) - return OrderedDict([("parameter_type", input_parameter_type), - ("optional", BooleanToolParameter(None, Element("param", name="optional", label="Optional", type="boolean", value=optional)))]) + + parameter_type_cond = Conditional() + parameter_type_cond.name = "parameter_definition" + parameter_type_cond.test_param = input_parameter_type + cases = [] + + for param_type in ["text", "integer", "float"]: + if param_type == "text": + if parameter_type == "text": + default = parameter_def.get("default") or "" + else: + default = "" + input_default_value = TextToolParameter(None, XML( + ''' + + + ''' + % default)) + elif param_type == "integer": + if parameter_type == "integer": + default = str(parameter_def.get("default") or "0") + else: + default = "0" + input_default_value = IntegerToolParameter(None, XML( + ''' + + + ''' + % default)) + elif param_type == "float": + if parameter_type == "float": + default = str(parameter_def.get("default") or "0.0") + else: + default = "0.0" + input_default_value = FloatToolParameter(None, XML( + ''' + + + ''' + % default)) + # color parameter defaults? + optional_value = BooleanToolParameter(None, Element("param", name="optional", label="Optional", type="boolean", checked=optional)) + optional_cond = Conditional() + optional_cond.name = "optional" + optional_cond.test_param = optional_value + + when_text = ConditionalWhen() + when_text.value = param_type + when_text.inputs = OrderedDict() + when_text.inputs["optional"] = optional_cond + + specify_default_checked = "default" in parameter_def + specify_default = BooleanToolParameter(None, Element("param", name="specify_default", label="Specify a default value", type="boolean", checked=specify_default_checked)) + specify_default_cond = Conditional() + specify_default_cond.name = "specify_default" + specify_default_cond.test_param = specify_default + + when_specify_default_true = ConditionalWhen() + when_specify_default_true.value = "true" + when_specify_default_true.inputs = OrderedDict() + when_specify_default_true.inputs["default"] = input_default_value + + when_specify_default_false = ConditionalWhen() + when_specify_default_false.value = "false" + when_specify_default_false.inputs = OrderedDict() + + specify_default_cond_cases = [when_specify_default_true, when_specify_default_false] + specify_default_cond.cases = specify_default_cond_cases + + when_true = ConditionalWhen() + when_true.value = "true" + when_true.inputs = OrderedDict() + when_true.inputs["default"] = specify_default_cond + + when_false = ConditionalWhen() + when_false.value = "false" + when_false.inputs = OrderedDict() + + optional_cases = [when_true, when_false] + optional_cond.cases = optional_cases + + cases.append(when_text) + + parameter_type_cond.cases = cases + return OrderedDict([("parameter_definition", parameter_type_cond)]) def get_runtime_inputs(self, **kwds): - parameter_type = self.state.inputs.get("parameter_type", self.default_parameter_type) - optional = self.state.inputs.get("optional", self.default_optional) + parameter_def = self._parse_state_into_dict() + parameter_type = parameter_def["parameter_type"] + optional = parameter_def["optional"] if parameter_type not in ["text", "boolean", "integer", "float", "color"]: raise ValueError("Invalid parameter type for workflow parameters encountered.") parameter_class = parameter_types[parameter_type] parameter_kwds = {} - if parameter_type in ["integer", "float"]: + + if optional: + default_value = parameter_def.get("default", self.default_default_value) + parameter_kwds["value"] = default_value + + if "value" not in parameter_kwds and parameter_type in ["integer", "float"]: parameter_kwds["value"] = str(0) # TODO: Use a dict-based description from YAML tool source - element = Element("param", name="input", label=self.label, type=parameter_type, optional=str(optional), **parameter_kwds) + element = Element("param", name="input", label=self.label, type=parameter_type, optional=optional, **parameter_kwds) input = parameter_class(None, element) return dict(input=input) @@ -708,18 +884,78 @@ class InputParameterModule(WorkflowModule): if data_only: return [] + parameter_def = self._parse_state_into_dict() return [dict( name='output', label=self.label, - type=self.state.inputs.get('parameter_type', self.parameter_type), + type=parameter_def.get('parameter_type'), parameter=True, )] def execute(self, trans, progress, invocation_step, use_cached_job=False): step = invocation_step.workflow_step - step_outputs = dict(output=step.state.inputs['input']) + input_value = step.state.inputs['input'] + if input_value is None: + default_value = safe_loads(step.tool_inputs.get("default", "{}")) + # TODO: look at parameter type and infer if value should be a dictionary + # instead. Guessing only field parameter types in CWL branch would have + # default as dictionary like this. + if not isinstance(default_value, dict): + default_value = {"value": default_value} + input_value = default_value.get("value", NO_REPLACEMENT) + step_outputs = dict(output=input_value) progress.set_outputs_for_input(invocation_step, step_outputs) + def step_state_to_tool_state(self, state): + state = safe_loads(state) + default_set, default_value = False, None + if "default" in state: + default_set = True + default_value = state["default"] + state["optional"] = True + state = { + "parameter_definition": { + "parameter_type": state["parameter_type"], + "optional": { + "optional": str(state.get("optional", False)) + } + } + } + if default_set: + state["parameter_definition"]["optional"]["specify_default"] = {} + state["parameter_definition"]["optional"]["specify_default"]["specify_default"] = True + state["parameter_definition"]["optional"]["specify_default"]["default"] = default_value + state = json.dumps(state) + return state + + def get_export_state(self): + return self._parse_state_into_dict() + + def _parse_state_into_dict(self): + inputs = self.state.inputs + rval = {} + # 19.0X tool state... + if "parameter_type" in inputs: + rval.update({"parameter_type": inputs["parameter_type"], "optional": False}) + # expanded tool state... + elif "parameter_definition" in inputs: + parameters_def = inputs["parameter_definition"] + if "optional" in parameters_def: + optional_state = parameters_def["optional"] + optional = bool(optional_state["optional"]) + if "specify_default" in optional_state and bool(optional_state["specify_default"]["specify_default"]): + rval["default"] = optional_state["specify_default"]["default"] + else: + optional = False + rval.update({"parameter_type": parameters_def["parameter_type"], "optional": optional}) + else: + rval.update({"parameter_type": self.default_parameter_type, "optional": self.default_optional}) + return rval + + def save_to_step(self, step): + step.type = self.type + step.tool_inputs = self._parse_state_into_dict() + class PauseModule(WorkflowModule): """ Initially this module will unconditionally pause a workflow - will aim @@ -1236,7 +1472,7 @@ class ToolModule(WorkflowModule): try: # Replace DummyDatasets with historydatasetassociations - visit_input_values(tool.inputs, execution_state.inputs, callback, no_replacement_value=NO_REPLACEMENT) + visit_input_values(tool.inputs, execution_state.inputs, callback, no_replacement_value=NO_REPLACEMENT, replace_optional_connections=True) except KeyError as k: message_template = "Error due to input mapping of '%s' in '%s'. A common cause of this is conditional outputs that cannot be determined until runtime, please review your workflow." message = message_template % (tool.name, k.message) diff --git a/lib/galaxy/workflow/run.py b/lib/galaxy/workflow/run.py index 57bf4aaa374..f3a6d5366f0 100644 --- a/lib/galaxy/workflow/run.py +++ b/lib/galaxy/workflow/run.py @@ -414,9 +414,13 @@ class WorkflowProgress(object): if self.inputs_by_step_id: step_id = step.id if step_id not in self.inputs_by_step_id and 'output' not in outputs: - template = "Step with id %s not found in inputs_step_id (%s)" - message = template % (step_id, self.inputs_by_step_id) - raise ValueError(message) + default_value = step.input_default_value + if default_value: + outputs['output'] = default_value + else: + template = "Step with id %s not found in inputs_step_id (%s)" + message = template % (step.log_str(), self.inputs_by_step_id) + raise ValueError(message) elif step_id in self.inputs_by_step_id: outputs['output'] = self.inputs_by_step_id[step_id] diff --git a/lib/galaxy/workflow/run_request.py b/lib/galaxy/workflow/run_request.py index ff495019438..2d08033c846 100644 --- a/lib/galaxy/workflow/run_request.py +++ b/lib/galaxy/workflow/run_request.py @@ -82,10 +82,17 @@ def _normalize_inputs(steps, inputs, inputs_by): for possible_input_key in possible_input_keys: if possible_input_key in inputs: inputs_key = possible_input_key - if not inputs_key: - message = "Workflow cannot be run because an expected input step '%s' has no input dataset." % step.id + default_value = step.tool_inputs.get("default") + optional = step.tool_inputs.get("optional") or False + # Need to be careful here to make sure 'default' has correct type - not sure how to do that + # but asserting 'optional' is definitely a bool and not a String->Bool or something is a good + # start to ensure tool state is being preserved and loaded in a type safe way. + assert isinstance(optional, bool) + if not inputs_key and default_value is None and not optional: + message = "Workflow cannot be run because an expected input step '%s' (%s) is not optional and no input." % (step.id, step.label) raise exceptions.MessageException(message) - normalized_inputs[step.id] = inputs[inputs_key] + if inputs_key: + normalized_inputs[step.id] = inputs[inputs_key] return normalized_inputs diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 889d27d599c..17a6deb9b89 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -21,6 +21,13 @@ from galaxy_test.base.workflow_fixtures import ( WORKFLOW_NESTED_RUNTIME_PARAMETER, WORKFLOW_NESTED_SIMPLE, WORKFLOW_ONE_STEP_DEFAULT, + WORKFLOW_OPTIONAL_FALSE_INPUT_COLLECTION, + WORKFLOW_OPTIONAL_FALSE_INPUT_DATA, + WORKFLOW_OPTIONAL_TRUE_INPUT_COLLECTION, + WORKFLOW_OPTIONAL_TRUE_INPUT_DATA, + WORKFLOW_PARAMETER_INPUT_INTEGER_DEFAULT, + WORKFLOW_PARAMETER_INPUT_INTEGER_OPTIONAL, + WORKFLOW_PARAMETER_INPUT_INTEGER_REQUIRED, WORKFLOW_RENAME_ON_INPUT, WORKFLOW_RUNTIME_PARAMETER_AFTER_PAUSE, WORKFLOW_WITH_CUSTOM_REPORT_1, @@ -493,8 +500,14 @@ test_data: uploaded_workflow_id = self.workflow_populator.simple_workflow("test_for_export") downloaded_workflow = self._download_workflow(uploaded_workflow_id) assert downloaded_workflow["name"] == "test_for_export" - assert len(downloaded_workflow["steps"]) == 3 - first_input = downloaded_workflow["steps"]["0"]["inputs"][0] + steps = downloaded_workflow["steps"] + assert len(steps) == 3 + assert "0" in steps + first_step = steps["0"] + self._assert_has_keys(first_step, "inputs", "outputs") + inputs = first_step["inputs"] + assert len(inputs) > 0, first_step + first_input = inputs[0] assert first_input["name"] == "WorkflowInput1" assert first_input["description"] == "input1 description" self._assert_has_keys(downloaded_workflow, "a_galaxy_workflow", "format-version", "annotation", "uuid", "steps") @@ -2128,7 +2141,62 @@ steps: self.wait_for_invocation_and_jobs(history_id, workflow_id, invocation_id) self._assert_history_job_count(history_id, 4) - def test_run_with_validated_parameter_connection_valid(self): + def test_run_with_optional_data_specified_to_multi_data(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(WORKFLOW_OPTIONAL_TRUE_INPUT_DATA, test_data=""" +input1: + value: 1.bed + type: File +""", history_id=history_id, wait=True, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert "CCDS989.1_cds_0_0_chr1_147962193_r" in content + + def test_run_with_optional_data_unspecified_to_multi_data(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(WORKFLOW_OPTIONAL_TRUE_INPUT_DATA, test_data={}, history_id=history_id, wait=True, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert "No input selected" in content + + def test_run_with_non_optional_data_unspecified_fails_invocation(self): + with self.dataset_populator.test_history() as history_id: + error = self._run_jobs(WORKFLOW_OPTIONAL_FALSE_INPUT_DATA, test_data={}, history_id=history_id, wait=False, assert_ok=False, expected_response=400) + self._assert_failed_on_non_optional_input(error, "input1") + + def test_run_with_optional_collection_specified(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(WORKFLOW_OPTIONAL_TRUE_INPUT_COLLECTION, test_data=""" +input1: + type: paired + name: the_dataset_pair + elements: + - identifier: forward + value: 1.fastq + type: File + - identifier: reverse + value: 1.fastq + type: File +""", history_id=history_id, wait=True, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert "GAATTGATCAGGACATAGGACAACTGTAGGCACCAT" in content + + def test_run_with_optional_collection_unspecified(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(WORKFLOW_OPTIONAL_TRUE_INPUT_COLLECTION, test_data={}, history_id=history_id, wait=True, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert "No input specified." in content + + def test_run_with_non_optional_collection_unspecified_fails_invocation(self): + with self.dataset_populator.test_history() as history_id: + error = self._run_jobs(WORKFLOW_OPTIONAL_FALSE_INPUT_COLLECTION, test_data={}, history_id=history_id, wait=False, assert_ok=False, expected_response=400) + self._assert_failed_on_non_optional_input(error, "input1") + + def _assert_failed_on_non_optional_input(self, error, input_name): + assert "err_msg" in error + err_msg = error["err_msg"] + assert input_name in err_msg + assert "is not optional and no input" in err_msg + + def test_run_with_validated_parameter_connection_optional(self): with self.dataset_populator.test_history() as history_id: run_summary = self._run_jobs(""" class: GalaxyWorkflow @@ -2150,6 +2218,48 @@ text_input: jobs = self._history_jobs(history_id) assert len(jobs) == 1 + def test_run_with_int_parameter(self): + with self.dataset_populator.test_history() as history_id: + failed = False + try: + self._run_jobs(WORKFLOW_PARAMETER_INPUT_INTEGER_REQUIRED, test_data=""" +data_input: + value: 1.bed + type: File +""", history_id=history_id, wait=True, assert_ok=True) + except AssertionError as e: + assert '(int_input) is not optional' in str(e) + failed = True + assert failed + self._run_jobs(WORKFLOW_PARAMETER_INPUT_INTEGER_REQUIRED, test_data=""" +data_input: + value: 1.bed + type: File +int_input: + value: 1 + type: raw +""", history_id=history_id, wait=True, assert_ok=True) + self.dataset_populator.wait_for_history(history_id, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert len(content.splitlines()) == 1, content + + self._run_jobs(WORKFLOW_PARAMETER_INPUT_INTEGER_OPTIONAL, test_data=""" +data_input: + value: 1.bed + type: File +""", history_id=history_id, wait=True, assert_ok=True) + + def test_run_with_validated_parameter_connection_default_values(self): + with self.dataset_populator.test_history() as history_id: + self._run_jobs(WORKFLOW_PARAMETER_INPUT_INTEGER_DEFAULT, test_data=""" +data_input: + value: 1.bed + type: File +""", history_id=history_id, wait=True, assert_ok=True) + self.dataset_populator.wait_for_history(history_id, assert_ok=True) + content = self.dataset_populator.get_history_dataset_content(history_id) + assert len(content.splitlines()) == 3, content + def test_run_with_validated_parameter_connection_invalid(self): with self.dataset_populator.test_history() as history_id: self._run_jobs(""" @@ -3415,7 +3525,7 @@ steps: state: r2: - text: "" -""", history_id=history_id, wait=False, expected_response=400) +""", history_id=history_id, wait=False, expected_response=400, assert_ok=False) def _run_validation_workflow_with_substitions(self, substitions): workflow = self.workflow_populator.load_workflow_from_resource("test_workflow_validation_1") diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index 4a6815c915c..1c1485845d6 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -906,6 +906,9 @@ class BaseWorkflowPopulator(object): invocation_response = workflow_populator.invoke_workflow_raw(workflow_id, workflow_request) api_asserts.assert_status_code_is(invocation_response, expected_response) invocation = invocation_response.json() + if expected_response != 200: + assert not assert_ok + return invocation invocation_id = invocation.get('id') if invocation_id: # Wait for workflow to become fully scheduled and then for all jobs diff --git a/lib/galaxy_test/base/workflow_fixtures.py b/lib/galaxy_test/base/workflow_fixtures.py index 55de9ccc7e7..33651b93ace 100644 --- a/lib/galaxy_test/base/workflow_fixtures.py +++ b/lib/galaxy_test/base/workflow_fixtures.py @@ -316,6 +316,144 @@ steps: """ +WORKFLOW_PARAMETER_INPUT_INTEGER_REQUIRED = """ +class: GalaxyWorkflow +inputs: + data_input: data + int_input: integer +steps: + random: + tool_id: random_lines1 + in: + input: data_input + num_lines: int_input + state: + seed_source: + seed_source_selector: set_seed + seed: asdf +""" + + +WORKFLOW_PARAMETER_INPUT_INTEGER_OPTIONAL = """ +class: GalaxyWorkflow +inputs: + data_input: data + int_input: + type: integer + optional: true +steps: + random: + tool_id: random_lines1 + in: + input: data_input + num_lines: int_input + state: + seed_source: + seed_source_selector: set_seed + seed: asdf +""" + + +WORKFLOW_PARAMETER_INPUT_INTEGER_DEFAULT = """ +class: GalaxyWorkflow +inputs: + data_input: data + int_input: + type: integer + default: 3 +steps: + random: + tool_id: random_lines1 + in: + input: data_input + num_lines: int_input + state: + seed_source: + seed_source_selector: set_seed + seed: asdf +""" + + +WORKFLOW_OPTIONAL_TRUE_INPUT_DATA = """ +class: GalaxyWorkflow +inputs: + input1: + type: data + optional: true +outputs: + out1: + outputSource: the_tool/out1 +steps: + the_tool: + tool_id: multi_data_optional + in: + input1: input1 + out: + out1: out1 +""" + + +# Same workflow but non-optional input +WORKFLOW_OPTIONAL_FALSE_INPUT_DATA = """ +class: GalaxyWorkflow +inputs: + input1: + type: data + optional: false +outputs: + out1: + outputSource: the_tool/out1 +steps: + the_tool: + tool_id: multi_data_optional + in: + input1: input1 + out: + out1: out1 +""" + + +WORKFLOW_OPTIONAL_TRUE_INPUT_COLLECTION = """ +class: GalaxyWorkflow +inputs: + input1: + type: collection + collection_type: paired + optional: true +outputs: + out1: + outputSource: the_tool/out1 +steps: + the_tool: + tool_id: collection_optional_param + in: + f1: input1 + out: + out1: out1 +""" + + +# Same workflow but non-optional input +WORKFLOW_OPTIONAL_FALSE_INPUT_COLLECTION = """ +class: GalaxyWorkflow +inputs: + input1: + type: collection + collection_type: paired + optional: false +outputs: + out1: + outputSource: the_tool/out1 +steps: + the_tool: + tool_id: collection_optional_param + in: + f1: input1 + out: + out1: out1 +""" + + WORKFLOW_RUNTIME_PARAMETER_SIMPLE = """ class: GalaxyWorkflow inputs: