Skip to content

Commit

Permalink
workflow save_as: catch error from child, show error in client
Browse files Browse the repository at this point in the history 
For the `save_workflow_as` function, catch the `DuplicatedIdentifierException` exception, and commit only after there is no exception.

On the front-end, show save as errors in editor error modal.

Fixes galaxyproject#17405
  • Loading branch information
@ahmedhamidawan
ahmedhamidawan committed Jan 31, 2024
1 parent 7df2d78 commit cb3ee59
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 39 deletions.
19 changes: 15 additions & 4 deletions client/src/components/Workflow/Editor/Index.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ import { hide_modal } from "@/layout/modal";
import { getAppRoot } from "@/onload/loadConfig";
import { useScopePointerStore } from "@/stores/scopePointerStore";
import { LastQueue } from "@/utils/promise-queue";
import { errorMessageAsString } from "@/utils/simple-error";
import { defaultPosition } from "./composables/useDefaultStepPosition";
import { fromSimple, toSimple } from "./modules/model";
Expand Down Expand Up @@ -555,7 +556,15 @@ export default {
this.hasChanges = false;
await this.routeToWorkflow(newId);
} catch (e) {
this.onWorkflowError("Saving workflow failed, please contact an administrator.");
if (create) {
throw e;
}
const errorHeading = `Saving workflow as '${rename_name}' failed`;
this.onWorkflowError(errorHeading, errorMessageAsString(e) || "Please contact an administrator.", {
Ok: () => {
this.hideModal();
},
});
}
},
onSaveAs() {
Expand Down Expand Up @@ -617,13 +626,15 @@ export default {
);
}
} catch (e) {
this.onWorkflowError("Creating workflow failed"),
e || "Please contact an administrator.",
this.onWorkflowError(
"Creating workflow failed",
errorMessageAsString(e) || "Please contact an administrator.",
{
Ok: () => {
this.hideModal();
},
};
}
);
}
},
nameValidate() {
Expand Down
64 changes: 35 additions & 29 deletions lib/galaxy/managers/workflows.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -676,13 +676,16 @@ def update_workflow_from_raw_description(
# Put parameters in workflow mode
trans.workflow_building_mode = workflow_building_modes.ENABLED
dry_run = workflow_update_options.dry_run
workflow, missing_tool_tups = self._workflow_from_raw_description(
trans,
raw_workflow_description,
workflow_update_options,
name=stored_workflow.name,
dry_run=dry_run,
)
try:
workflow, missing_tool_tups = self._workflow_from_raw_description(
trans,
raw_workflow_description,
workflow_update_options,
name=stored_workflow.name,
dry_run=dry_run,
)
except exceptions.DuplicatedIdentifierException as e:
raise e

if missing_tool_tups and not workflow_update_options.allow_missing_tools:
errors = []
Expand Down Expand Up @@ -794,29 +797,32 @@ def _workflow_from_raw_description(
)
subworkflow_id_map[key] = subworkflow

# Keep track of tools required by the workflow that are not available in
# the local Galaxy instance. Each tuple in the list of missing_tool_tups
# will be (tool_id, tool_name, tool_version, step_id).
missing_tool_tups = []
for step_dict in self.__walk_step_dicts(data):
if not dry_run:
self.__load_subworkflows(
trans, step_dict, subworkflow_id_map, workflow_state_resolution_options, dry_run=dry_run
)
try:
# Keep track of tools required by the workflow that are not available in
# the local Galaxy instance. Each tuple in the list of missing_tool_tups
# will be (tool_id, tool_name, tool_version, step_id).
missing_tool_tups = []
for step_dict in self.__walk_step_dicts(data):
if not dry_run:
self.__load_subworkflows(
trans, step_dict, subworkflow_id_map, workflow_state_resolution_options, dry_run=dry_run
)

module_kwds = workflow_state_resolution_options.dict()
module_kwds.update(kwds) # TODO: maybe drop this?
for step_dict in self.__walk_step_dicts(data):
module, step = self.__module_from_dict(trans, steps, steps_by_external_id, step_dict, **module_kwds)
if isinstance(module, ToolModule) and module.tool is None:
missing_tool_tup = (module.tool_id, module.get_name(), module.tool_version, step_dict["id"])
if missing_tool_tup not in missing_tool_tups:
missing_tool_tups.append(missing_tool_tup)
if module.get_errors():
workflow.has_errors = True

# Second pass to deal with connections between steps
self.__connect_workflow_steps(steps, steps_by_external_id, dry_run)
module_kwds = workflow_state_resolution_options.dict()
module_kwds.update(kwds) # TODO: maybe drop this?
for step_dict in self.__walk_step_dicts(data):
module, step = self.__module_from_dict(trans, steps, steps_by_external_id, step_dict, **module_kwds)
if isinstance(module, ToolModule) and module.tool is None:
missing_tool_tup = (module.tool_id, module.get_name(), module.tool_version, step_dict["id"])
if missing_tool_tup not in missing_tool_tups:
missing_tool_tups.append(missing_tool_tup)
if module.get_errors():
workflow.has_errors = True

# Second pass to deal with connections between steps
self.__connect_workflow_steps(steps, steps_by_external_id, dry_run)
except exceptions.DuplicatedIdentifierException as e:
raise e

workflow.has_cycles = True
workflow.steps = steps
Expand Down
22 changes: 16 additions & 6 deletions lib/galaxy/webapps/galaxy/controllers/workflow.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from sqlalchemy.sql import expression

from galaxy import (
exceptions,
model,
util,
web,
Expand Down Expand Up @@ -416,11 +417,6 @@ def save_workflow_as(
workflow_annotation = sanitize_html(workflow_annotation)
self.add_item_annotation(trans.sa_session, trans.get_user(), stored_workflow, workflow_annotation)

# Persist
session = trans.sa_session
session.add(stored_workflow)
with transaction(session):
session.commit()
workflow_update_options = WorkflowUpdateOptions(
update_stored_workflow_attributes=False, # taken care of above
from_tool_form=from_tool_form,
Expand All @@ -432,15 +428,29 @@ def save_workflow_as(
workflow_data,
workflow_update_options,
)
except exceptions.DuplicatedIdentifierException as e:
return self.message_exception(trans, e.err_msg, False)
except MissingToolsException as e:
return dict(
missing_tools_return = dict(
name=e.workflow.name,
message=(
"This workflow includes missing or invalid tools. "
"It cannot be saved until the following steps are removed or the missing tools are enabled."
),
errors=e.errors,
)
else:
missing_tools_return = None

# Persist
session = trans.sa_session
session.add(stored_workflow)
with transaction(session):
session.commit()

if missing_tools_return:
return missing_tools_return

return trans.security.encode_id(stored_workflow.id)
else:
# This is an error state, 'save as' must have a workflow_name
Expand Down

0 comments on commit cb3ee59

Please sign in to comment.