From 6be4e2bdbd99b82de4b741e72056556e38dc2ff4 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 31 Dec 2020 12:30:13 -0500 Subject: [PATCH] Rework refactoring API for better separation of concerns. Thinner controller, pydantic model to separate options available for manager (which has to render a response) from those of the refactoring executor. --- lib/galaxy/managers/workflows.py | 12 +++++++++++- lib/galaxy/webapps/galaxy/api/workflows.py | 7 ++----- lib/galaxy/workflow/refactor/execute.py | 4 ++-- lib/galaxy/workflow/refactor/schema.py | 2 +- test/integration/test_workflow_refactoring.py | 6 +++--- test/unit/workflows/test_refactor_models.py | 4 ++-- 6 files changed, 21 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 5166f8be33d..2219cd1ee2f 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -45,6 +45,7 @@ from galaxy.workflow.modules import ( WorkflowModuleInjector ) from galaxy.workflow.refactor.execute import WorkflowRefactorExecutor +from galaxy.workflow.refactor.schema import RefactorActions from galaxy.workflow.reports import generate_report from galaxy.workflow.resources import get_resource_mapper_function from galaxy.workflow.steps import attach_ordered_steps @@ -1408,7 +1409,7 @@ class WorkflowContentsManager(UsesAnnotations): if default_label and util.unicodify(default_label).lower() not in ['input dataset', 'input dataset collection']: step.label = module.label = default_label - def refactor(self, trans, stored_workflow, refactor_request): + def do_refactor(self, trans, stored_workflow, refactor_request): """Apply supplied actions to stored_workflow.latest_workflow to build a new version. """ workflow = stored_workflow.latest_workflow @@ -1434,6 +1435,15 @@ class WorkflowContentsManager(UsesAnnotations): workflow_update_options, ) + def refactor(self, trans, stored_workflow, refactor_request): + stored_workflow, errors = self.do_refactor(trans, stored_workflow, refactor_request) + # TODO: handle errors... + return self.workflow_to_dict(trans, stored_workflow, style=refactor_request.style) + + +class RefactorRequest(RefactorActions): + style: str = "export" + class WorkflowStateResolutionOptions(BaseModel): # fill in default tool state when updating, may change tool_state diff --git a/lib/galaxy/webapps/galaxy/api/workflows.py b/lib/galaxy/webapps/galaxy/api/workflows.py index 17fa308a68b..a7de9ed1f77 100644 --- a/lib/galaxy/webapps/galaxy/api/workflows.py +++ b/lib/galaxy/webapps/galaxy/api/workflows.py @@ -24,6 +24,7 @@ from galaxy.managers import ( from galaxy.managers.jobs import fetch_job_states, invocation_job_source_iter, summarize_job_metrics from galaxy.managers.workflows import ( MissingToolsException, + RefactorRequest, WorkflowCreateOptions, WorkflowUpdateOptions, ) @@ -49,7 +50,6 @@ from galaxy.webapps.base.controller import ( ) from galaxy.workflow.extract import extract_workflow from galaxy.workflow.modules import module_factory -from galaxy.workflow.refactor.schema import RefactorRequest from galaxy.workflow.run import invoke, queue_invoke from galaxy.workflow.run_request import build_workflow_run_configs @@ -656,12 +656,9 @@ class WorkflowsAPIController(BaseAPIController, UsesStoredWorkflowMixin, UsesAnn """ stored_workflow = self.__get_stored_workflow(trans, id, **kwds) refactor_request = RefactorRequest(**payload) - style = payload.get("style", "export") - result, errors = self.workflow_contents_manager.refactor( + return self.workflow_contents_manager.refactor( trans, stored_workflow, refactor_request ) - # TODO: handle errors... - return self.workflow_contents_manager.workflow_to_dict(trans, stored_workflow, style=style) @expose_api def build_module(self, trans, payload=None): diff --git a/lib/galaxy/workflow/refactor/execute.py b/lib/galaxy/workflow/refactor/execute.py index 70120711685..13f84a4fba3 100644 --- a/lib/galaxy/workflow/refactor/execute.py +++ b/lib/galaxy/workflow/refactor/execute.py @@ -22,7 +22,7 @@ from .schema import ( FillStepDefaultsAction, InputReferenceByOrderIndex, OutputReferenceByOrderIndex, - RefactorRequest, + RefactorActions, RemoveUnlabeledWorkflowOutputs, step_reference_union, StepReferenceByLabel, @@ -51,7 +51,7 @@ class WorkflowRefactorExecutor: self.workflow = workflow self.module_injector = module_injector - def refactor(self, refactor_request: RefactorRequest): + def refactor(self, refactor_request: RefactorActions): for action in refactor_request.actions: action_type = action.action_type refactor_method_name = "_apply_%s" % action_type diff --git a/lib/galaxy/workflow/refactor/schema.py b/lib/galaxy/workflow/refactor/schema.py index b7565e884bb..58ce21270f3 100644 --- a/lib/galaxy/workflow/refactor/schema.py +++ b/lib/galaxy/workflow/refactor/schema.py @@ -196,6 +196,6 @@ for action_class in union_action_classes.__args__: # type: ignore ACTION_CLASSES_BY_TYPE[action_type] = action_class -class RefactorRequest(BaseModel): +class RefactorActions(BaseModel): actions: List[Action] dry_run: bool = False diff --git a/test/integration/test_workflow_refactoring.py b/test/integration/test_workflow_refactoring.py index 815115e835f..439ff5907a1 100644 --- a/test/integration/test_workflow_refactoring.py +++ b/test/integration/test_workflow_refactoring.py @@ -1,7 +1,7 @@ import json from galaxy.managers.context import ProvidesAppContext -from galaxy.workflow.refactor.schema import RefactorRequest +from galaxy.workflow.refactor.schema import RefactorActions from galaxy_test.base.populators import ( WorkflowPopulator, ) @@ -320,10 +320,10 @@ steps: def _refactor(self, actions): user = self._app.model.session.query(self._app.model.User).order_by(self._app.model.User.id.desc()).limit(1).one() mock_trans = MockTrans(self._app, user) - return self._manager.refactor( + return self._manager.do_refactor( mock_trans, self._most_recent_stored_workflow, - RefactorRequest(**{"actions": actions}) + RefactorActions(**{"actions": actions}) ) @property diff --git a/test/unit/workflows/test_refactor_models.py b/test/unit/workflows/test_refactor_models.py index 9bb65aa2ab4..05deee750ab 100644 --- a/test/unit/workflows/test_refactor_models.py +++ b/test/unit/workflows/test_refactor_models.py @@ -1,4 +1,4 @@ -from galaxy.workflow.refactor.schema import RefactorRequest +from galaxy.workflow.refactor.schema import RefactorActions def test_root_list(): @@ -16,7 +16,7 @@ def test_root_list(): {"action_type": "extract_legacy_parameter", "name": "foo", "label": "new_foo"}, ], } - ar = RefactorRequest(**request) + ar = RefactorActions(**request) actions = ar.actions a0 = actions[0]