From e6aeb21a08e4404c6d220ca8246238b654fbf40b Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 26 Aug 2022 17:49:50 +0200 Subject: [PATCH 1/6] Always call strip() on data_column column values Not just when they start with `c`. Should fix running workflows that were created by manually writing columns in the text area field and then hitting enter. --- lib/galaxy/tools/parameters/basic.py | 10 +++++++-- lib/galaxy_test/api/test_workflows.py | 30 ++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 1b3e147f74c..26d10f37d7f 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1259,6 +1259,11 @@ class ColumnListParameter(SelectToolParameter): self.is_dynamic = True self.usecolnames = input_source.get_bool("use_header_names", False) + def to_json(self, value, app, use_security): + if isinstance(value, str): + return value.strip() + return value + def from_json(self, value, trans, other_values=None): """ Label convention prepends column number with a 'c', but tool uses the integer. This @@ -1292,8 +1297,9 @@ class ColumnListParameter(SelectToolParameter): @staticmethod def _strip_c(column): if isinstance(column, str): - if column.startswith('c') and len(column) > 1 and all(c.isdigit() for c in column[1:]): - column = column.strip().lower()[1:] + column = column.strip() + if column.startswith("c") and len(column) > 1 and all(c.isdigit() for c in column[1:]): + column = column.lower()[1:] return column def get_column_list(self, trans, other_values): diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 6f26a060ca4..76aaf2ef8a9 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -1319,7 +1319,35 @@ steps: col_names: 'B' """, history_id=history_id) - @skip_without_tool('column_param') + @skip_without_tool("column_param_list") + def test_comma_separated_columns_with_trailing_newline(self): + # Tests that workflows with weird tool state continue to run. + # In this case the newline may have been added by the workflow editor + # text field that is used for data_column parameters + with self.dataset_populator.test_history() as history_id: + job_summary = self._run_workflow( + """class: GalaxyWorkflow +steps: + empty_output: + tool_id: empty_output + outputs: + out_file1: + change_datatype: tabular + column_param_list: + tool_id: column_param_list + in: + input1: empty_output/out_file1 + state: + col: '2,3\n' + col_names: 'B\n' +""", + history_id=history_id, + ) + job = self.dataset_populator.get_job_details(job_summary.jobs[0]["id"], full=True).json() + assert "col 2,3" in job["command_line"] + assert 'echo "col_names B" >>' in job["command_line"] + + @skip_without_tool("column_param") def test_runtime_data_column_parameter(self): with self.dataset_populator.test_history() as history_id: self._run_jobs("""class: GalaxyWorkflow From a01c8c5a88debc7f9e0c7e2865a33acb9633acab Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 3 Nov 2022 10:37:49 +0100 Subject: [PATCH 2/6] Use _run_jobs instead of _run_workflows --- lib/galaxy_test/api/test_workflows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 76aaf2ef8a9..4e719cb6bc9 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -1325,7 +1325,7 @@ steps: # In this case the newline may have been added by the workflow editor # text field that is used for data_column parameters with self.dataset_populator.test_history() as history_id: - job_summary = self._run_workflow( + job_summary = self._run_jobs( """class: GalaxyWorkflow steps: empty_output: From ae0a98342d35309eef1077db94280053e44fd4ae Mon Sep 17 00:00:00 2001 From: Alireza Heidari Date: Wed, 2 Nov 2022 15:45:23 +0100 Subject: [PATCH 3/6] Fix displaying named tags --- templates/tagging_common.mako | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/tagging_common.mako b/templates/tagging_common.mako index a4d95adf1cd..cbaad93c12c 100644 --- a/templates/tagging_common.mako +++ b/templates/tagging_common.mako @@ -77,7 +77,7 @@ from markupsafe import escape item_tag_names = [] for ta in item_tags: - item_tag_names.append(escape(ta.tag.name)) + item_tag_names.append(escape(f"#{ta.value}" if ta.value else ta.tag.name)) %>
From dc528e7283180bd5451d90936b1819f67f5714d0 Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 4 Nov 2022 10:26:02 -0400 Subject: [PATCH 4/6] Update migration instructions --- lib/tool_shed/webapp/model/migrate/check.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/tool_shed/webapp/model/migrate/check.py b/lib/tool_shed/webapp/model/migrate/check.py index 10df4878017..98ab037ccfe 100644 --- a/lib/tool_shed/webapp/model/migrate/check.py +++ b/lib/tool_shed/webapp/model/migrate/check.py @@ -27,14 +27,14 @@ migrate_repository = repository.Repository(migrate_repository_directory) def create_or_verify_database(url, engine_options=None): """ - Check that the database is use-able, possibly creating it if empty (this is + Check that the database is useable, possibly creating it if empty (this is the only time we automatically create tables, otherwise we force the user to do it using the management script so they can create backups). 1) Empty database --> initialize with latest version and return 2) Database older than migration support --> fail and require manual update 3) Database at state where migrate support introduced --> add version control information but make no changes (might still require manual update) - 4) Database versioned but out of date --> fail with informative message, user must run "sh manage_db.sh upgrade" + 4) Database versioned but out of date --> fail with informative message, user must run "sh manage_toolshed_db.sh upgrade" """ engine_options = engine_options or {} @@ -81,7 +81,7 @@ def create_or_verify_database(url, engine_options=None): migrate_repository.versions.latest, ) exception_msg += "Back up your database and then migrate the schema by running the following from your Galaxy installation directory:" - exception_msg += "\n\nsh manage_db.sh upgrade tool_shed\n" + exception_msg += "\n\nsh manage_toolshed_db.sh upgrade\n" raise Exception(exception_msg) else: log.info("At database version %d" % db_schema.version) From a216d1e8e3e64aab6979199df92a37036976c151 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 3 Nov 2022 20:01:04 +0100 Subject: [PATCH 5/6] Fix broken subworkflow reodering on workflow import We started reordering workflow steps on import in https://github.com/galaxyproject/galaxy/pull/13641. If subworkflow steps are reordered before the input connections are set the wrong subworkflow steps are referenced when we build the workflow connection. This broke the IWC tests for gromacs. We now do not reorder subworkflow steps until the parent workflow is being reordered, which is after all connections are set up. --- lib/galaxy/managers/workflows.py | 23 ++++++++--- lib/galaxy/workflow/extract.py | 3 +- lib/galaxy/workflow/steps.py | 6 ++- lib/galaxy_test/api/test_workflows.py | 59 +++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 111730f93b9..626a2b2a4f5 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -572,6 +572,7 @@ class WorkflowContentsManager(UsesAnnotations): source=None, add_to_menu=False, hidden=False, + is_subworkflow=False, ): data = raw_workflow_description.as_dict # Put parameters in workflow mode @@ -591,6 +592,7 @@ class WorkflowContentsManager(UsesAnnotations): raw_workflow_description, workflow_create_options, name=name, + is_subworkflow=is_subworkflow, ) if "uuid" in data: workflow.uuid = data["uuid"] @@ -680,7 +682,7 @@ class WorkflowContentsManager(UsesAnnotations): return workflow, errors def _workflow_from_raw_description( - self, trans, raw_workflow_description, workflow_state_resolution_options, name, **kwds + self, trans, raw_workflow_description, workflow_state_resolution_options, name, is_subworkflow=False, **kwds ): # don't commit the workflow or attach its part to the sa session - just build a # a transient model to operate on or render. @@ -755,8 +757,12 @@ class WorkflowContentsManager(UsesAnnotations): # Second pass to deal with connections between steps self.__connect_workflow_steps(steps, steps_by_external_id, dry_run) - # Order the steps if possible - attach_ordered_steps(workflow, steps) + workflow.has_cycles = True + workflow.steps = steps + # we can't reorder subworkflows, as step connections would become invalid + if not is_subworkflow: + # Order the steps if possible + attach_ordered_steps(workflow) return workflow, missing_tool_tups @@ -848,7 +854,7 @@ class WorkflowContentsManager(UsesAnnotations): """ if len(workflow.steps) == 0: raise exceptions.MessageException("Workflow cannot be run because it does not have any steps.") - if attach_ordered_steps(workflow, workflow.steps): + if attach_ordered_steps(workflow): raise exceptions.MessageException("Workflow cannot be run because it contains cycles.") trans.workflow_building_mode = workflow_building_modes.USE_HISTORY module_injector = WorkflowModuleInjector(trans) @@ -940,7 +946,7 @@ class WorkflowContentsManager(UsesAnnotations): """ if len(workflow.steps) == 0: raise exceptions.MessageException("Workflow cannot be run because it does not have any steps.") - if attach_ordered_steps(workflow, workflow.steps): + if attach_ordered_steps(workflow): raise exceptions.MessageException("Workflow cannot be run because it contains cycles.") # Ensure that the user has a history @@ -1637,6 +1643,7 @@ class WorkflowContentsManager(UsesAnnotations): steps.append(step) external_id = step_dict["id"] steps_by_external_id[external_id] = step + step.order_index = external_id if "workflow_outputs" in step_dict: workflow_outputs = step_dict["workflow_outputs"] found_output_names = set() @@ -1703,7 +1710,11 @@ class WorkflowContentsManager(UsesAnnotations): def __build_embedded_subworkflow(self, trans, data, workflow_state_resolution_options): raw_workflow_description = self.ensure_raw_description(data) subworkflow = self.build_workflow_from_raw_description( - trans, raw_workflow_description, workflow_state_resolution_options, hidden=True + trans, + raw_workflow_description, + workflow_state_resolution_options, + hidden=True, + is_subworkflow=True, ).workflow return subworkflow diff --git a/lib/galaxy/workflow/extract.py b/lib/galaxy/workflow/extract.py index b5f8a6e56f4..4e0f102e388 100644 --- a/lib/galaxy/workflow/extract.py +++ b/lib/galaxy/workflow/extract.py @@ -52,8 +52,9 @@ def extract_workflow( # Workflow to populate workflow = model.Workflow() workflow.name = workflow_name + workflow.steps = steps # Order the steps if possible - attach_ordered_steps(workflow, steps) + attach_ordered_steps(workflow) # And let's try to set up some reasonable locations on the canvas # (these are pretty arbitrary values) levorder = order_workflow_steps_with_levels(steps) diff --git a/lib/galaxy/workflow/steps.py b/lib/galaxy/workflow/steps.py index a87e1828728..b632504b42f 100644 --- a/lib/galaxy/workflow/steps.py +++ b/lib/galaxy/workflow/steps.py @@ -10,11 +10,11 @@ from galaxy.util.topsort import ( ) -def attach_ordered_steps(workflow, steps): +def attach_ordered_steps(workflow): """Attempt to topologically order steps and attach to workflow. If this fails - the workflow contains cycles so it mark it as such. """ - ordered_steps = order_workflow_steps(steps) + ordered_steps = order_workflow_steps(workflow.steps) workflow.has_cycles = True if ordered_steps: workflow.has_cycles = False @@ -30,6 +30,8 @@ def order_workflow_steps(steps): """ position_data_available = bool(steps) for step in steps: + if step.subworkflow: + attach_ordered_steps(step.subworkflow) if not step.position or "left" not in step.position or "top" not in step.position: position_data_available = False if position_data_available: diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 1b318e69e5e..8573cadb24e 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -5021,6 +5021,65 @@ steps: run_workflow_response = self.workflow_populator.invoke_workflow_raw(uploaded_workflow_id, workflow_request) return run_workflow_response, history_id + def test_subworkflow_import_order_maintained(self): + summary = self._run_workflow( + """ +class: GalaxyWorkflow +inputs: + outer_input_1: + type: int + default: 1 + position: + left: 0 + top: 0 + outer_input_2: + type: int + default: 2 + position: + left: 100 + top: 0 +steps: + nested_workflow: + in: + inner_input_1: outer_input_1 + inner_input_2: outer_input_2 + run: + class: GalaxyWorkflow + inputs: + inner_input_1: + type: int + position: + left: 100 + top: 0 + inner_input_2: + type: int + position: + left: 0 + top: 0 + steps: [] + outputs: + - label: nested_out_1 + outputSource: inner_input_1/output + - label: nested_out_2 + outputSource: inner_input_2/output +outputs: + - label: out_1 + outputSource: nested_workflow/nested_out_1 + - label: out_2 + outputSource: nested_workflow/nested_out_2 +""", + assert_ok=False, + wait=False, + ) + self.workflow_populator.wait_for_invocation(summary.workflow_id, summary.invocation_id) + self.workflow_populator.wait_for_history_workflows( + summary.history_id, assert_ok=False, expected_invocation_count=2 + ) + invocation = self.workflow_populator.get_invocation(summary.invocation_id) + output_values = invocation["output_values"] + assert output_values["out_1"] == 1 + assert output_values["out_2"] == 2 + @skip_without_tool("random_lines1") def test_run_replace_params_by_steps(self): workflow_request, history_id, workflow_id, steps = self._setup_random_x2_workflow_steps( From 494bae8dddbb9de3bc99a808ce45be01bebe9f2f Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 5 Nov 2022 18:14:12 +0100 Subject: [PATCH 6/6] Revert "Use _run_jobs instead of _run_workflows" This reverts commit a01c8c5a88debc7f9e0c7e2865a33acb9633acab. _run_workflow has the right type annotations, but 21.09 didn't have this. --- lib/galaxy_test/api/test_workflows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index f5babd3e333..af60f7f644c 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -1279,7 +1279,7 @@ steps: # In this case the newline may have been added by the workflow editor # text field that is used for data_column parameters with self.dataset_populator.test_history() as history_id: - job_summary = self._run_jobs( + job_summary = self._run_workflow( """class: GalaxyWorkflow steps: empty_output: