From bbe90dd3f759158a7b8fa84ffe3803b9702427d7 Mon Sep 17 00:00:00 2001 From: Simon Bray Date: Wed, 9 Feb 2022 11:55:28 +0100 Subject: [PATCH 1/5] block workflow invocation if wrong tool versions installed --- .../Workflow/Run/WorkflowRunForm.vue | 3 ++ .../Workflow/Run/WorkflowRunFormSimple.vue | 1 + lib/galaxy/managers/workflows.py | 13 ++++---- lib/galaxy/webapps/galaxy/api/workflows.py | 30 +++++++++++++++---- lib/galaxy_test/api/test_workflows.py | 29 ++++++++++++++---- 5 files changed, 59 insertions(+), 17 deletions(-) diff --git a/client/src/components/Workflow/Run/WorkflowRunForm.vue b/client/src/components/Workflow/Run/WorkflowRunForm.vue index 8b63094e327..a65441c99dd 100644 --- a/client/src/components/Workflow/Run/WorkflowRunForm.vue +++ b/client/src/components/Workflow/Run/WorkflowRunForm.vue @@ -206,6 +206,9 @@ export default { // Tool form always wants a list of invocations back // so that inputs can be batched. batch: true, + // the user is already warned if tool versions are wrong, + // they can still choose to invoke the workflow anyway. + require_exact_tool_versions: false, }; console.debug("WorkflowRunForm::onExecute()", "Ready for submission.", jobDef); diff --git a/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue b/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue index e95f828c33a..6d8543f0e66 100644 --- a/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue +++ b/client/src/components/Workflow/Run/WorkflowRunFormSimple.vue @@ -114,6 +114,7 @@ export default { inputs_by: "step_index", batch: true, use_cached_job: this.useJobCache, + require_exact_tool_versions: false, }; if (this.targetHistory == "current") { data.history_id = this.model.historyId; diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 1d5769e86bb..e1539a3bebd 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -1506,15 +1506,16 @@ class WorkflowContentsManager(UsesAnnotations): dry_run=refactor_request.dry_run, ) - def get_all_tool_ids(self, workflow): - tool_ids = set() + def get_all_tools(self, workflow): + tools = [] for step in workflow.steps: if step.type == 'tool': if step.tool_id: - tool_ids.add(step.tool_id) - elif step.type == 'subworkflow': - tool_ids.update(self.get_all_tool_ids(step.subworkflow)) - return tool_ids + if {"tool_id": step.tool_id, "tool_version": step.tool_version} not in tools: + tools.append({"tool_id": step.tool_id, "tool_version": step.tool_version}) + elif step.type == "subworkflow": + tools.extend(self.get_all_tools(step.subworkflow)) + return tools class RefactorRequest(RefactorActions): diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 5e06952caea..793de027e55 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -191,8 +191,12 @@ class WorkflowsAPIController(BaseGalaxyAPIController, UsesStoredWorkflowMixin, U workflows = [] workflows_by_toolshed = dict() for value in rval: - tool_ids = self.workflow_contents_manager.get_all_tool_ids(self.__get_stored_workflow(trans, value['id']).latest_workflow) - missing_tool_ids = [tool_id for tool_id in tool_ids if self.app.toolbox.is_missing_shed_tool(tool_id)] + tools = self.workflow_contents_manager.get_all_tools( + self.__get_stored_workflow(trans, value["id"]).latest_workflow + ) + missing_tool_ids = [ + tool["tool_id"] for tool in tools if self.app.toolbox.is_missing_shed_tool(tool["tool_id"]) + ] if len(missing_tool_ids) > 0: value['missing_tools'] = missing_tool_ids workflows_missing_tools.append(value) @@ -752,10 +756,24 @@ class WorkflowsAPIController(BaseGalaxyAPIController, UsesStoredWorkflowMixin, U if not is_batch and len(run_configs) != 1: raise exceptions.RequestParameterInvalidException("Must specify 'batch' to use batch parameters.") - tool_ids = self.workflow_contents_manager.get_all_tool_ids(workflow) - missing_tool_ids = [tool_id for tool_id in tool_ids if not self.app.toolbox.has_tool(tool_id)] - if missing_tool_ids: - raise exceptions.MessageException(f"Workflow was not invoked; the following required tools are not installed: {', '.join(missing_tool_ids)}") + require_exact_tool_versions = util.string_as_bool(payload.get("require_exact_tool_versions", "true")) + tools = self.workflow_contents_manager.get_all_tools(workflow) + missing_tools = [ + tool + for tool in tools + if not self.app.toolbox.has_tool( + tool["tool_id"], tool_version=tool["tool_version"], exact=require_exact_tool_versions + ) + ] + if missing_tools: + missing_tools_message = "Workflow was not invoked; the following required tools are not installed: " + if require_exact_tool_versions: + missing_tools_message += ", ".join( + [f"{tool['tool_id']} (version {tool['tool_version']})" for tool in missing_tools] + ) + else: + missing_tools_message += ", ".join([tool["tool_id"] for tool in missing_tools]) + raise exceptions.MessageException(missing_tools_message) invocations = [] for run_config in run_configs: diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 416f7dc31b3..8b355ebe6ff 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -20,6 +20,7 @@ from galaxy_test.base.populators import ( wait_on, WorkflowPopulator ) +from galaxy_test.base.uses_shed import UsesShed from galaxy_test.base.workflow_fixtures import ( WORKFLOW_NESTED_REPLACEMENT_PARAMETER, WORKFLOW_NESTED_RUNTIME_PARAMETER, @@ -208,8 +209,7 @@ input1: # - Allow post to workflows//run in addition to posting to # /workflows with id in payload. # - Much more testing obviously, always more testing. -class WorkflowsApiTestCase(BaseWorkflowsApiTestCase, ChangeDatatypeTestCase): - +class WorkflowsApiTestCase(BaseWorkflowsApiTestCase, ChangeDatatypeTestCase, UsesShed): def test_show_valid(self): workflow_id = self.workflow_populator.simple_workflow("dummy") workflow_id = self.workflow_populator.simple_workflow("test_regular") @@ -876,15 +876,34 @@ steps: assert invocation["state"] == "scheduled", invocation def test_run_workflow_with_missing_tool(self): + self.install_repository("iuc", "compose_text_param", "feb3acba1e0a") # 0.1.0 with self.dataset_populator.test_history() as history_id: workflow_id = self._upload_yaml_workflow(""" class: GalaxyWorkflow steps: - step1: + nonexistent: tool_id: nonexistent_tool tool_version: "0.1" -""") - invocation_response = self.__invoke_workflow(workflow_id, history_id=history_id, assert_ok=False) + label: nonexistent + compose_text_param: + tool_id: compose_text_param + tool_version: 0.0.1 + label: compose_text_param +""" + ) + # should fail and return both tool ids since version 0.0.1 of compose_text_param does not exist + invocation_response = self.__invoke_workflow( + workflow_id, history_id=history_id, assert_ok=False, request={"require_exact_tool_versions": True} + ) + self._assert_status_code_is(invocation_response, 400) + self.assertEqual( + invocation_response.json().get("err_msg"), + "Workflow was not invoked; the following required tools are not installed: nonexistent_tool (version 0.1), compose_text_param (version 0.0.1)", + ) + # should fail but return only the tool_id of non_existent tool as another version of compose_text_param is installed + invocation_response = self.__invoke_workflow( + workflow_id, history_id=history_id, assert_ok=False, request={"require_exact_tool_versions": False} + ) self._assert_status_code_is(invocation_response, 400) self.assertEqual(invocation_response.json().get('err_msg'), "Workflow was not invoked; the following required tools are not installed: nonexistent_tool") From 8078dab412d9c1fc61d4286dc5f073fca2897aec Mon Sep 17 00:00:00 2001 From: Simon Bray Date: Wed, 9 Feb 2022 15:15:39 +0100 Subject: [PATCH 2/5] fix small bug in toolbox's get_tool() so non-shed tool can be used for missing tool workflow invocation test --- lib/galaxy/tool_util/toolbox/base.py | 2 +- lib/galaxy_test/api/test_workflows.py | 16 +++++++--------- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/galaxy/tool_util/toolbox/base.py b/lib/galaxy/tool_util/toolbox/base.py index 734971aea46..eed2134d34a 100644 --- a/lib/galaxy/tool_util/toolbox/base.py +++ b/lib/galaxy/tool_util/toolbox/base.py @@ -594,7 +594,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): return self._tools_by_id[tool_id] elif tool_version in self._tool_versions_by_id[tool_id]: return self._tool_versions_by_id[tool_id][tool_version] - elif exact: + if exact: # We're looking for an exact match, so we skip lineage and # versionless mapping, though we may want to check duplicate # toolsheds diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 8b355ebe6ff..7d7fc24b903 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -20,7 +20,6 @@ from galaxy_test.base.populators import ( wait_on, WorkflowPopulator ) -from galaxy_test.base.uses_shed import UsesShed from galaxy_test.base.workflow_fixtures import ( WORKFLOW_NESTED_REPLACEMENT_PARAMETER, WORKFLOW_NESTED_RUNTIME_PARAMETER, @@ -209,7 +208,7 @@ input1: # - Allow post to workflows//run in addition to posting to # /workflows with id in payload. # - Much more testing obviously, always more testing. -class WorkflowsApiTestCase(BaseWorkflowsApiTestCase, ChangeDatatypeTestCase, UsesShed): +class WorkflowsApiTestCase(BaseWorkflowsApiTestCase, ChangeDatatypeTestCase): def test_show_valid(self): workflow_id = self.workflow_populator.simple_workflow("dummy") workflow_id = self.workflow_populator.simple_workflow("test_regular") @@ -876,7 +875,6 @@ steps: assert invocation["state"] == "scheduled", invocation def test_run_workflow_with_missing_tool(self): - self.install_repository("iuc", "compose_text_param", "feb3acba1e0a") # 0.1.0 with self.dataset_populator.test_history() as history_id: workflow_id = self._upload_yaml_workflow(""" class: GalaxyWorkflow @@ -886,21 +884,21 @@ steps: tool_version: "0.1" label: nonexistent compose_text_param: - tool_id: compose_text_param - tool_version: 0.0.1 - label: compose_text_param + tool_id: multiple_versions + tool_version: 0.3 + label: multiple_versions """ ) - # should fail and return both tool ids since version 0.0.1 of compose_text_param does not exist + # should fail and return both tool ids since version 0.3 of multiple_versions does not exist invocation_response = self.__invoke_workflow( workflow_id, history_id=history_id, assert_ok=False, request={"require_exact_tool_versions": True} ) self._assert_status_code_is(invocation_response, 400) self.assertEqual( invocation_response.json().get("err_msg"), - "Workflow was not invoked; the following required tools are not installed: nonexistent_tool (version 0.1), compose_text_param (version 0.0.1)", + "Workflow was not invoked; the following required tools are not installed: nonexistent_tool (version 0.1), multiple_versions (version 0.3)", ) - # should fail but return only the tool_id of non_existent tool as another version of compose_text_param is installed + # should fail but return only the tool_id of non_existent tool as another version of multiple_versions is installed invocation_response = self.__invoke_workflow( workflow_id, history_id=history_id, assert_ok=False, request={"require_exact_tool_versions": False} ) From 7f8a198af377a6ce27838107bd12a94114a33786 Mon Sep 17 00:00:00 2001 From: Simon Bray Date: Thu, 10 Feb 2022 11:28:17 +0100 Subject: [PATCH 3/5] revert change to toolbox's get_tool() --- lib/galaxy/tool_util/toolbox/base.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tool_util/toolbox/base.py b/lib/galaxy/tool_util/toolbox/base.py index eed2134d34a..70642dbf7c8 100644 --- a/lib/galaxy/tool_util/toolbox/base.py +++ b/lib/galaxy/tool_util/toolbox/base.py @@ -594,7 +594,9 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): return self._tools_by_id[tool_id] elif tool_version in self._tool_versions_by_id[tool_id]: return self._tool_versions_by_id[tool_id][tool_version] - if exact: + # should be if exact=True not elif? Otherwise we can end up doing non-exact searches even + # if exact=True. Anyway, changing it breaks a lot of tests involving built-in tools + elif exact: # We're looking for an exact match, so we skip lineage and # versionless mapping, though we may want to check duplicate # toolsheds From 50636056f4be0378d4878535d5ccd43ad079c48f Mon Sep 17 00:00:00 2001 From: Simon Bray Date: Thu, 10 Feb 2022 11:30:36 +0100 Subject: [PATCH 4/5] add integration test for invoking workflows with nonexistent tool_ids or tool_versions --- test/integration/test_workflow_invocation.py | 55 ++++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 test/integration/test_workflow_invocation.py diff --git a/test/integration/test_workflow_invocation.py b/test/integration/test_workflow_invocation.py new file mode 100644 index 00000000000..f8d877207e6 --- /dev/null +++ b/test/integration/test_workflow_invocation.py @@ -0,0 +1,55 @@ +"""Integration tests for workflow syncing.""" + +from galaxy_test.base.populators import ( + DatasetPopulator, + WorkflowPopulator, +) +from galaxy_test.base.uses_shed import UsesShed +from galaxy_test.driver import integration_util + + +class WorkflowInvocationTestCase(integration_util.IntegrationTestCase, UsesShed): + + framework_tool_and_types = True + require_admin_user = False + + def setUp(self): + super().setUp() + self.dataset_populator = DatasetPopulator(self.galaxy_interactor) + self.workflow_populator = WorkflowPopulator(self.galaxy_interactor) + + def test_run_workflow_with_missing_tool(self): + self.install_repository("iuc", "compose_text_param", "feb3acba1e0a") # 0.1.0 + with self.dataset_populator.test_history() as history_id: + workflow_id = self.workflow_populator.upload_yaml_workflow( + """ +class: GalaxyWorkflow +steps: + nonexistent: + tool_id: nonexistent_tool + tool_version: "0.1" + label: nonexistent + compose_text_param: + tool_id: compose_text_param + tool_version: "0.0.1" + label: compose_text_param +""" + ) + # should fail and return both tool ids since version 0.0.1 of compose_text_param does not exist + invocation_response = self.workflow_populator.invoke_workflow( + workflow_id, history_id=history_id, assert_ok=False, request={"require_exact_tool_versions": True} + ) + self._assert_status_code_is(invocation_response, 400) + self.assertEqual( + invocation_response.json().get("err_msg"), + "Workflow was not invoked; the following required tools are not installed: nonexistent_tool (version 0.1), compose_text_param (version 0.0.1)", + ) + # should fail but return only the tool_id of non_existent tool as another version of compose_text_param is installed + invocation_response = self.workflow_populator.invoke_workflow( + workflow_id, history_id=history_id, assert_ok=False, request={"require_exact_tool_versions": False} + ) + self._assert_status_code_is(invocation_response, 400) + self.assertEqual( + invocation_response.json().get("err_msg"), + "Workflow was not invoked; the following required tools are not installed: nonexistent_tool", + ) From cc79e028ed77143ad77c7f857316f86f86d983ed Mon Sep 17 00:00:00 2001 From: Simon Bray Date: Thu, 10 Feb 2022 17:04:09 +0100 Subject: [PATCH 5/5] remove test_run_workflow_with_missing_tool() from API tests --- lib/galaxy_test/api/test_workflows.py | 31 --------------------------- 1 file changed, 31 deletions(-) diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 7d7fc24b903..0be1fb889af 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -874,37 +874,6 @@ steps: invocation = self._invocation_details(workflow_id, invocation_id) assert invocation["state"] == "scheduled", invocation - def test_run_workflow_with_missing_tool(self): - with self.dataset_populator.test_history() as history_id: - workflow_id = self._upload_yaml_workflow(""" -class: GalaxyWorkflow -steps: - nonexistent: - tool_id: nonexistent_tool - tool_version: "0.1" - label: nonexistent - compose_text_param: - tool_id: multiple_versions - tool_version: 0.3 - label: multiple_versions -""" - ) - # should fail and return both tool ids since version 0.3 of multiple_versions does not exist - invocation_response = self.__invoke_workflow( - workflow_id, history_id=history_id, assert_ok=False, request={"require_exact_tool_versions": True} - ) - self._assert_status_code_is(invocation_response, 400) - self.assertEqual( - invocation_response.json().get("err_msg"), - "Workflow was not invoked; the following required tools are not installed: nonexistent_tool (version 0.1), multiple_versions (version 0.3)", - ) - # should fail but return only the tool_id of non_existent tool as another version of multiple_versions is installed - invocation_response = self.__invoke_workflow( - workflow_id, history_id=history_id, assert_ok=False, request={"require_exact_tool_versions": False} - ) - self._assert_status_code_is(invocation_response, 400) - self.assertEqual(invocation_response.json().get('err_msg'), "Workflow was not invoked; the following required tools are not installed: nonexistent_tool") - @skip_without_tool("collection_creates_pair") def test_workflow_run_output_collections(self) -> None: with self.dataset_populator.test_history() as history_id: