Skip to content

Commit

Permalink
Fix broken subworkflow reodering on workflow import
Browse files Browse the repository at this point in the history
We started reordering workflow steps on import in
galaxyproject#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.
  • Loading branch information
mvdbeek committed Nov 3, 2022
1 parent b2f0bf1 commit 86f42eb
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 6 deletions.
19 changes: 15 additions & 4 deletions lib/galaxy/managers/workflows.py
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,7 @@ def build_workflow_from_raw_description(
source=None,
add_to_menu=False,
hidden=False,
is_subworkflow=False,
):
data = raw_workflow_description.as_dict
# Put parameters in workflow mode
Expand All @@ -591,6 +592,7 @@ def build_workflow_from_raw_description(
raw_workflow_description,
workflow_create_options,
name=name,
is_subworkflow=is_subworkflow,
)
if "uuid" in data:
workflow.uuid = data["uuid"]
Expand Down Expand Up @@ -680,7 +682,7 @@ def update_workflow_from_raw_description(
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.
Expand Down Expand Up @@ -755,8 +757,12 @@ def _workflow_from_raw_description(
# 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

Expand Down Expand Up @@ -1637,6 +1643,7 @@ def __module_from_dict(
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()
Expand Down Expand Up @@ -1703,7 +1710,11 @@ def __load_subworkflow_from_step_dict(
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

Expand Down
6 changes: 4 additions & 2 deletions lib/galaxy/workflow/steps.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
)


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
Expand All @@ -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:
Expand Down

0 comments on commit 86f42eb

Please sign in to comment.