From 8a8acc625fcea935f0c7ca178b993de2caacc579 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 23 Aug 2022 13:42:05 +0200 Subject: [PATCH] Fix refactor action dropping outputs on subworkflow connection I thought it would be cleaner to make a distinction between the output name and the output label when reasoning about a subworkflow step in the context of a parent workflow (https://github.com/galaxyproject/galaxy/pull/14473/commits/b0727be9e0b096a2dacacb767a1f4ac0fc3e6540#diff-a50bcc5c08ade11249449c4100ea8c286727df601b52a5b9588eb2a385d97da7R543), in case we'd later want to do something with the label. I don't think I thought about this correctly though, and it breaks connections between steps that were made by referencing the subworkflow's output name. This wasn't necessary for the main goal of https://github.com/galaxyproject/galaxy/pull/14473, so I'm partially reverting that here. Fixes https://github.com/galaxyproject/planemo/pull/1262#issuecomment-1223874571 --- lib/galaxy/workflow/modules.py | 15 ++++++--------- lib/galaxy/workflow/refactor/execute.py | 2 +- lib/galaxy_test/selenium/test_workflow_editor.py | 4 ++-- test/unit/workflows/test_modules.py | 4 ++-- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 635fef8dec0..f24120b6863 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -540,7 +540,12 @@ class SubWorkflowModule(WorkflowModule): data_output["name"] == workflow_output["output_name"] or data_output_uuid == workflow_output_uuid ): - data_output["label"] = label + change_datatype_action = step["post_job_actions"].get( + f"ChangeDatatypeAction{data_output['name']}" + ) + if change_datatype_action: + self.post_job_actions[f"ChangeDatatypeAction{label}"] = change_datatype_action + data_output["name"] = label # That's the right data_output break else: @@ -551,15 +556,7 @@ class SubWorkflowModule(WorkflowModule): f"Workflow output '{workflow_output['output_name']}' defined, but not listed among data outputs" ) continue - post_job_actions = step["post_job_actions"].copy() - change_datatype_action = post_job_actions.pop(f"ChangeDatatypeAction{data_output['name']}", None) - # Post job actions are referred to by tool output name, - # but that's not guaranteed to be unique within a workflow, - # but the label is unique. - if change_datatype_action: - post_job_actions[f"ChangeDatatypeAction{label}"] = change_datatype_action - self.post_job_actions.update(post_job_actions) outputs.append(data_output) return outputs diff --git a/lib/galaxy/workflow/refactor/execute.py b/lib/galaxy/workflow/refactor/execute.py index 76681506b95..2ba45ffc5d2 100644 --- a/lib/galaxy/workflow/refactor/execute.py +++ b/lib/galaxy/workflow/refactor/execute.py @@ -517,7 +517,7 @@ class WorkflowRefactorExecutor: # TODO: find workflow outputs that need to be dropped and report them upgrade_inputs = step.module.get_all_inputs() upgrade_outputs = step.module.get_all_outputs() - upgrade_output_names = [u.get("label") or u["name"] for u in upgrade_outputs] + upgrade_output_names = {u["name"] for u in upgrade_outputs} upgrade_order_index = step_def["id"] upgrade_label = step_def.get("label") all_input_connections = step_def.get("input_connections") diff --git a/lib/galaxy_test/selenium/test_workflow_editor.py b/lib/galaxy_test/selenium/test_workflow_editor.py index 8a4b5cf139a..f39b4357011 100644 --- a/lib/galaxy_test/selenium/test_workflow_editor.py +++ b/lib/galaxy_test/selenium/test_workflow_editor.py @@ -587,8 +587,8 @@ steps: node.output_data_row(output_name="workflow_output", extension="bam").wait_for_visible() # Move canvas, so terminals are in viewport self.move_center_of_canvas(xoffset=100, yoffset=100) - self.workflow_editor_connect("nested_workflow#out_file1", "metadata_bam#input_bam") - self.assert_connected("nested_workflow#out_file1", "metadata_bam#input_bam") + self.workflow_editor_connect("nested_workflow#workflow_output", "metadata_bam#input_bam") + self.assert_connected("nested_workflow#workflow_output", "metadata_bam#input_bam") @selenium_test def test_editor_duplicate_node(self): diff --git a/test/unit/workflows/test_modules.py b/test/unit/workflows/test_modules.py index b72ee0c1e9e..77ad7802146 100644 --- a/test/unit/workflows/test_modules.py +++ b/test/unit/workflows/test_modules.py @@ -244,9 +244,9 @@ def test_subworkflow_new_outputs(): outputs = subworkflow_module.get_data_outputs() assert len(outputs) == 2, len(outputs) output1, output2 = outputs - assert output1["label"] == "out1" + assert output1["name"] == "out1" assert output1["extensions"] == ["input"] - assert output2["label"] == "4:out_file1", output2["label"] + assert output2["name"] == "4:out_file1", output2["name"] class MapOverTestCase(NamedTuple):