From 5050ccde3df7113846e83d567d38194d783f26e2 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 27 Jun 2024 14:54:55 +0200 Subject: [PATCH 01/11] Add test to make sure when_expression is not lost on refactoring --- test/integration/test_workflow_refactoring.py | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/integration/test_workflow_refactoring.py b/test/integration/test_workflow_refactoring.py index 6a03d818ce31..d2501090aeef 100644 --- a/test/integration/test_workflow_refactoring.py +++ b/test/integration/test_workflow_refactoring.py @@ -537,6 +537,33 @@ def test_tool_version_upgrade_no_state_change(self): assert len(action_executions[0].messages) == 0 assert self._latest_workflow.step_by_label("the_step").tool_version == "0.2" + def test_tool_version_upgrade_keeps_when_expression(self): + self.workflow_populator.upload_yaml_workflow( + """ +class: GalaxyWorkflow +inputs: + the_bool: + type: boolean +steps: + the_step: + tool_id: multiple_versions + tool_version: '0.1' + state: + inttest: 0 + when: $inputs.the_bool +""" + ) + assert self._latest_workflow.step_by_label("the_step").tool_version == "0.1" + actions: ActionsJson = [ + {"action_type": "upgrade_tool", "step": {"label": "the_step"}}, + ] + action_executions = self._refactor(actions).action_executions + assert len(action_executions) == 1 + assert len(action_executions[0].messages) == 0 + step = self._latest_workflow.step_by_label("the_step") + assert step.tool_version == "0.2" + assert step.when_expression + def test_tool_version_upgrade_state_added(self): self.workflow_populator.upload_yaml_workflow( """ From ed8dc4b8e29db41993316bf4a65591e73f264219 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 27 Jun 2024 14:54:23 +0200 Subject: [PATCH 02/11] Fix dropped when_expression on workflow/tool_upgrade Fixes https://github.com/galaxyproject/galaxy/issues/18441 --- lib/galaxy/workflow/refactor/execute.py | 3 +++ test/integration/test_workflow_refactoring.py | 4 +++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/workflow/refactor/execute.py b/lib/galaxy/workflow/refactor/execute.py index d35cbbbc7785..2582380fc623 100644 --- a/lib/galaxy/workflow/refactor/execute.py +++ b/lib/galaxy/workflow/refactor/execute.py @@ -531,6 +531,9 @@ def _patch_step(self, execution, step, step_def): if upgrade_input["name"] == input_name: matching_input = upgrade_input break + elif step.when_expression and f"inputs.{input_name}" in step.when_expression: + # TODO: eventually track step inputs more formally + matching_input = upgrade_input # In the future check parameter type, format, mapping status... if matching_input is None: diff --git a/test/integration/test_workflow_refactoring.py b/test/integration/test_workflow_refactoring.py index d2501090aeef..d68b8916145b 100644 --- a/test/integration/test_workflow_refactoring.py +++ b/test/integration/test_workflow_refactoring.py @@ -548,9 +548,11 @@ def test_tool_version_upgrade_keeps_when_expression(self): the_step: tool_id: multiple_versions tool_version: '0.1' + in: + when: the_bool state: inttest: 0 - when: $inputs.the_bool + when: $(inputs.when) """ ) assert self._latest_workflow.step_by_label("the_step").tool_version == "0.1" From 851f9c82e52300de0e1b9a0e68ed49927d33ca02 Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 28 Jun 2024 11:33:29 +0200 Subject: [PATCH 03/11] Refactor WorkflowContentsManager to use a list for tags instead of a string --- lib/galaxy/managers/workflows.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/managers/workflows.py b/lib/galaxy/managers/workflows.py index 7404c78854a7..b54afd169066 100644 --- a/lib/galaxy/managers/workflows.py +++ b/lib/galaxy/managers/workflows.py @@ -1413,7 +1413,7 @@ def _workflow_to_dict_export(self, trans, stored=None, workflow=None, internal=F If `allow_upgrade`, the workflow and sub-workflows might use updated tool versions when refactoring. """ annotation_str = "" - tag_str = "" + tags_list = [] annotation_owner = None if stored is not None: if stored.id: @@ -1424,7 +1424,7 @@ def _workflow_to_dict_export(self, trans, stored=None, workflow=None, internal=F or self.get_item_annotation_str(trans.sa_session, annotation_owner, stored) or "" ) - tag_str = stored.make_tag_string_list() + tags_list = stored.make_tag_string_list() else: # dry run with flushed workflow objects, just use the annotation annotations = stored.annotations @@ -1437,7 +1437,7 @@ def _workflow_to_dict_export(self, trans, stored=None, workflow=None, internal=F data["format-version"] = "0.1" data["name"] = workflow.name data["annotation"] = annotation_str - data["tags"] = tag_str + data["tags"] = tags_list if workflow.uuid is not None: data["uuid"] = str(workflow.uuid) steps: Dict[int, Dict[str, Any]] = {} From 959252073778d04857c3086b67a2bc3fa4fc4fa8 Mon Sep 17 00:00:00 2001 From: Arash Date: Fri, 28 Jun 2024 13:39:21 +0200 Subject: [PATCH 04/11] Add test --- lib/galaxy_test/api/test_workflows.py | 7 + .../base/data/test_subworkflow_with_tags.ga | 134 ++++++++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 lib/galaxy_test/base/data/test_subworkflow_with_tags.ga diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 36549c820015..810269b570b1 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -7613,6 +7613,13 @@ def _all_user_invocation_ids(self): invocation_ids = [i["id"] for i in all_invocations_for_user.json()] return invocation_ids + def test_subworkflow_tags(self): + workflow = self.workflow_populator.load_workflow_from_resource("test_subworkflow_with_tags") + workflow_id = self.workflow_populator.create_workflow(workflow) + downloaded_workflow = self._download_workflow(workflow_id) + subworkflow = downloaded_workflow["steps"]["1"]["subworkflow"] + assert subworkflow["tags"] == [] + class TestAdminWorkflowsApi(BaseWorkflowsApiTestCase): require_admin_user = True diff --git a/lib/galaxy_test/base/data/test_subworkflow_with_tags.ga b/lib/galaxy_test/base/data/test_subworkflow_with_tags.ga new file mode 100644 index 000000000000..9b5fb172320a --- /dev/null +++ b/lib/galaxy_test/base/data/test_subworkflow_with_tags.ga @@ -0,0 +1,134 @@ +{ + "a_galaxy_workflow": "true", + "annotation": "Test main ", + "comments": [], + "format-version": "0.1", + "name": "Unnamed Workflow", + "report": { + "markdown": "\n# Workflow Execution Report\n\n## Workflow Inputs\n```galaxy\ninvocation_inputs()\n```\n\n## Workflow Outputs\n```galaxy\ninvocation_outputs()\n```\n\n## Workflow\n```galaxy\nworkflow_display()\n```\n" + }, + "steps": { + "0": { + "annotation": "", + "content_id": null, + "errors": null, + "id": 0, + "input_connections": {}, + "inputs": [], + "label": null, + "name": "Input dataset", + "outputs": [], + "position": { + "left": 0, + "top": 0 + }, + "tool_id": null, + "tool_state": "{\"optional\": false, \"tag\": null}", + "tool_version": null, + "type": "data_input", + "uuid": "967583e9-d2a6-444a-ba31-6fb749d03f9e", + "when": null, + "workflow_outputs": [] + }, + "1": { + "annotation": "", + "id": 1, + "input_connections": { + "0:Input dataset": { + "id": 0, + "input_subworkflow_step_id": 0, + "output_name": "output" + } + }, + "inputs": [], + "label": null, + "name": "Workflow with tags", + "outputs": [], + "position": { + "left": 249, + "top": 51 + }, + "subworkflow": { + "a_galaxy_workflow": "true", + "annotation": "", + "comments": [], + "format-version": "0.1", + "name": "Workflow with tags", + "report": { + "markdown": "\n# Workflow Execution Report\n\n## Workflow Inputs\n```galaxy\ninvocation_inputs()\n```\n\n## Workflow Outputs\n```galaxy\ninvocation_outputs()\n```\n\n## Workflow\n```galaxy\nworkflow_display()\n```\n" + }, + "steps": { + "0": { + "annotation": "", + "content_id": null, + "errors": null, + "id": 0, + "input_connections": {}, + "inputs": [], + "label": null, + "name": "Input dataset", + "outputs": [], + "position": { + "left": 0, + "top": 0.0 + }, + "tool_id": null, + "tool_state": "{\"optional\": false, \"tag\": null}", + "tool_version": null, + "type": "data_input", + "uuid": "eca9b088-ff50-4253-8387-01512f03ff2f", + "when": null, + "workflow_outputs": [] + }, + "1": { + "annotation": "", + "content_id": "addValue", + "errors": null, + "id": 1, + "input_connections": { + "input": { + "id": 0, + "output_name": "output" + } + }, + "inputs": [ + { + "description": "runtime parameter for tool Add column", + "name": "input" + } + ], + "label": null, + "name": "Add column", + "outputs": [ + { + "name": "out_file1", + "type": "input" + } + ], + "position": { + "left": 123, + "top": 112.0 + }, + "post_job_actions": {}, + "tool_id": "addValue", + "tool_state": "{\"exp\": \"1\", \"input\": {\"__class__\": \"RuntimeValue\"}, \"iterate\": \"no\", \"__page__\": null, \"__rerun_remap_job_id__\": null}", + "tool_version": "1.0.0", + "type": "tool", + "uuid": "7016e754-149b-402a-bb17-eb6cd4b1ab0a", + "when": null, + "workflow_outputs": [] + } + }, + "uuid": "c33370a9-188f-4af8-bfcc-137c577c79ba" + }, + "tool_id": null, + "type": "subworkflow", + "uuid": "90bdcd13-418a-49da-847e-02926942bf4b", + "when": null, + "workflow_outputs": [] + } + }, + "tags": [], + "uuid": "64d7fac3-6402-412b-9db6-490ffc18e129", + "version": 1 +} \ No newline at end of file From e69d7b2f8b32369d754aea336102f5dfc2254f1c Mon Sep 17 00:00:00 2001 From: Jonathan Laperle Date: Fri, 28 Jun 2024 08:01:25 -0400 Subject: [PATCH 05/11] Prevent deleted users from reseting password --- lib/galaxy/managers/users.py | 2 ++ test/unit/app/managers/test_UserManager.py | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index 59dc13184c3b..18f7adf40123 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -613,6 +613,8 @@ def send_reset_email(self, trans, payload, **kwd): def get_reset_token(self, trans, email): reset_user = get_user_by_email(trans.sa_session, email, self.app.model.User) + if reset_user.deleted: + return None, None if not reset_user and email != email.lower(): reset_user = self._get_user_by_email_case_insensitive(trans.sa_session, email) if reset_user: diff --git a/test/unit/app/managers/test_UserManager.py b/test/unit/app/managers/test_UserManager.py index 871daac25c8f..c160a39bd2da 100644 --- a/test/unit/app/managers/test_UserManager.py +++ b/test/unit/app/managers/test_UserManager.py @@ -232,6 +232,16 @@ def validate_send_email(frm, to, subject, body, config, html=None): mock_unique_id.assert_called_once() assert result is None + def test_reset_email_user_deleted(self): + self.trans.app.config.allow_user_deletion = True + self.log("should not produce the password reset email if user is deleted") + user_email = "user@nopassword.com" + user = self.user_manager.create(email=user_email, username="nopassword") + self.user_manager.delete(user) + assert user.deleted is True + message = self.user_manager.send_reset_email(self.trans, {"email": user_email}) + assert message == "Failed to produce password reset token. User not found." + def test_get_user_by_identity(self): # return None if username/email not found assert self.user_manager.get_user_by_identity("xyz") is None From 7eaa54a05b9340f6435fd5b4706b807ec5b549f8 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 28 Jun 2024 15:28:46 +0200 Subject: [PATCH 06/11] Add input extra files to `get_input_fnames` This should fix volume mounts in kubernetes, which calls: ``` volume_mounts.extend(generate_relative_mounts(data_claim, job_wrapper.job_io.get_input_fnames())) ``` The only other consumers are pbs, using an undocument config option (ahem ...). --- lib/galaxy/job_execution/setup.py | 20 ++++++++++++-------- lib/galaxy/jobs/splitters/basic.py | 2 +- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/job_execution/setup.py b/lib/galaxy/job_execution/setup.py index 934dd528e74a..c241c5cc0b71 100644 --- a/lib/galaxy/job_execution/setup.py +++ b/lib/galaxy/job_execution/setup.py @@ -213,22 +213,26 @@ def get_input_dataset_fnames(self, ds: DatasetInstance) -> List[str]: for value in ds.metadata.values(): if isinstance(value, MetadataFile): filenames.append(value.get_file_name()) + if ds.dataset and ds.dataset.extra_files_path_exists(): + filenames.append(ds.dataset.extra_files_path) return filenames - def get_input_fnames(self) -> List[str]: + def get_input_datasets(self) -> List[DatasetInstance]: job = self.job + return [ + da.dataset for da in job.input_datasets + job.input_library_datasets if da.dataset + ] # da is JobToInputDatasetAssociation object + + def get_input_fnames(self) -> List[str]: filenames = [] - for da in job.input_datasets + job.input_library_datasets: # da is JobToInputDatasetAssociation object - if da.dataset: - filenames.extend(self.get_input_dataset_fnames(da.dataset)) + for ds in self.get_input_datasets(): + filenames.extend(self.get_input_dataset_fnames(ds)) return filenames def get_input_paths(self) -> List[DatasetPath]: - job = self.job paths = [] - for da in job.input_datasets + job.input_library_datasets: # da is JobToInputDatasetAssociation object - if da.dataset: - paths.append(self.get_input_path(da.dataset)) + for ds in self.get_input_datasets(): + paths.append(self.get_input_path(ds)) return paths def get_input_path(self, dataset: DatasetInstance) -> DatasetPath: diff --git a/lib/galaxy/jobs/splitters/basic.py b/lib/galaxy/jobs/splitters/basic.py index f19f0bd81636..f2c804ce8ceb 100644 --- a/lib/galaxy/jobs/splitters/basic.py +++ b/lib/galaxy/jobs/splitters/basic.py @@ -13,7 +13,7 @@ def set_basic_defaults(job_wrapper): def do_split(job_wrapper): - if len(job_wrapper.job_io.get_input_fnames()) > 1 or len(job_wrapper.job_io.get_output_fnames()) > 1: + if len(job_wrapper.job_io.get_input_datasets()) > 1 or len(job_wrapper.job_io.get_output_fnames()) > 1: log.error("The basic splitter is not capable of handling jobs with multiple inputs or outputs.") raise Exception("Job Splitting Failed, the basic splitter only handles tools with one input and one output") # add in the missing information for splitting the one input and merging the one output From a475dfd8d871e8ddac2db6c9774a4563ffa5203c Mon Sep 17 00:00:00 2001 From: Jonathan Laperle Date: Fri, 28 Jun 2024 12:10:52 -0400 Subject: [PATCH 07/11] Update lib/galaxy/managers/users.py Co-authored-by: John Davis --- lib/galaxy/managers/users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index 18f7adf40123..82e949b34ff4 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -613,7 +613,7 @@ def send_reset_email(self, trans, payload, **kwd): def get_reset_token(self, trans, email): reset_user = get_user_by_email(trans.sa_session, email, self.app.model.User) - if reset_user.deleted: + if reset_user and reset_user.deleted: return None, None if not reset_user and email != email.lower(): reset_user = self._get_user_by_email_case_insensitive(trans.sa_session, email) From beb705428af8c493a499141645017b41c5311ba5 Mon Sep 17 00:00:00 2001 From: Jonathan Laperle Date: Fri, 28 Jun 2024 12:50:05 -0400 Subject: [PATCH 08/11] fix indentation --- test/unit/app/managers/test_UserManager.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/unit/app/managers/test_UserManager.py b/test/unit/app/managers/test_UserManager.py index c160a39bd2da..b8286838d103 100644 --- a/test/unit/app/managers/test_UserManager.py +++ b/test/unit/app/managers/test_UserManager.py @@ -233,14 +233,14 @@ def validate_send_email(frm, to, subject, body, config, html=None): assert result is None def test_reset_email_user_deleted(self): - self.trans.app.config.allow_user_deletion = True - self.log("should not produce the password reset email if user is deleted") - user_email = "user@nopassword.com" - user = self.user_manager.create(email=user_email, username="nopassword") - self.user_manager.delete(user) - assert user.deleted is True - message = self.user_manager.send_reset_email(self.trans, {"email": user_email}) - assert message == "Failed to produce password reset token. User not found." + self.trans.app.config.allow_user_deletion = True + self.log("should not produce the password reset email if user is deleted") + user_email = "user@nopassword.com" + user = self.user_manager.create(email=user_email, username="nopassword") + self.user_manager.delete(user) + assert user.deleted is True + message = self.user_manager.send_reset_email(self.trans, {"email": user_email}) + assert message == "Failed to produce password reset token. User not found." def test_get_user_by_identity(self): # return None if username/email not found From 2579e8119688f47bae148daa0f49084377ea1cf5 Mon Sep 17 00:00:00 2001 From: Jonathan Laperle Date: Sat, 29 Jun 2024 04:33:54 -0400 Subject: [PATCH 09/11] combine user exist check with user deleted check --- lib/galaxy/managers/users.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/galaxy/managers/users.py b/lib/galaxy/managers/users.py index 82e949b34ff4..2196857cf479 100644 --- a/lib/galaxy/managers/users.py +++ b/lib/galaxy/managers/users.py @@ -613,11 +613,9 @@ def send_reset_email(self, trans, payload, **kwd): def get_reset_token(self, trans, email): reset_user = get_user_by_email(trans.sa_session, email, self.app.model.User) - if reset_user and reset_user.deleted: - return None, None if not reset_user and email != email.lower(): reset_user = self._get_user_by_email_case_insensitive(trans.sa_session, email) - if reset_user: + if reset_user and not reset_user.deleted: prt = self.app.model.PasswordResetToken(reset_user) trans.sa_session.add(prt) with transaction(trans.sa_session): From 95df247985e04fe2d536ff035a4767b9f67ad43d Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Mon, 1 Jul 2024 10:25:03 +0200 Subject: [PATCH 10/11] 22.05 pin bleach to 5.0.1 22.05 is currently failing planemo tests because bleach 5.0.0 has an error in its requirements that has been fixed here https://github.com/mozilla/bleach/pull/655 --- lib/galaxy/dependencies/dev-requirements.txt | 2 +- lib/galaxy/dependencies/pinned-requirements.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/dependencies/dev-requirements.txt b/lib/galaxy/dependencies/dev-requirements.txt index 2de873dbd204..e68fe4b8916e 100644 --- a/lib/galaxy/dependencies/dev-requirements.txt +++ b/lib/galaxy/dependencies/dev-requirements.txt @@ -25,7 +25,7 @@ beaker==1.11.0 billiard==3.6.4.0; python_version >= "3.7" bioblend==0.17.0; python_version >= "3.7" black==22.3.0; python_full_version >= "3.6.2" -bleach==5.0.0; python_version >= "3.7" +bleach==5.0.1; python_version >= "3.7" boltons==21.0.0 boto==2.49.0 bx-python==0.8.13; python_version >= "3.7" diff --git a/lib/galaxy/dependencies/pinned-requirements.txt b/lib/galaxy/dependencies/pinned-requirements.txt index 9e4d35bb5bdc..5d014d17ec54 100644 --- a/lib/galaxy/dependencies/pinned-requirements.txt +++ b/lib/galaxy/dependencies/pinned-requirements.txt @@ -20,7 +20,7 @@ bdbag==1.6.3; (python_version >= "2.7" and python_full_version < "3.0.0") or (py beaker==1.11.0 billiard==3.6.4.0; python_version >= "3.7" bioblend==0.17.0; python_version >= "3.7" -bleach==5.0.0; python_version >= "3.7" +bleach==5.0.1; python_version >= "3.7" boltons==21.0.0 boto==2.49.0 bx-python==0.8.13; python_version >= "3.7" From c4ee734f3969bc766f3d6c5a7525a5f401061a03 Mon Sep 17 00:00:00 2001 From: Sanjay Kumar Srikakulam Date: Fri, 28 Jun 2024 11:59:08 +0000 Subject: [PATCH 11/11] update plausible script name in mako templates --- lib/tool_shed/webapp/templates/galaxy_client_app.mako | 2 +- templates/galaxy_client_app.mako | 2 +- templates/js-app.mako | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/tool_shed/webapp/templates/galaxy_client_app.mako b/lib/tool_shed/webapp/templates/galaxy_client_app.mako index a8ed317e27bf..846fcc7f60f9 100644 --- a/lib/tool_shed/webapp/templates/galaxy_client_app.mako +++ b/lib/tool_shed/webapp/templates/galaxy_client_app.mako @@ -72,7 +72,7 @@ ${ h.dumps( dictionary, indent=( 2 if trans.debug else 0 ) ) } <%def name="config_plausible_analytics(plausible_server, plausible_domain)"> %if plausible_server and plausible_domain: - + %else: %endif diff --git a/templates/galaxy_client_app.mako b/templates/galaxy_client_app.mako index 504b3300e2b1..caec938bbe89 100644 --- a/templates/galaxy_client_app.mako +++ b/templates/galaxy_client_app.mako @@ -73,7 +73,7 @@ ${ h.dumps( dictionary, indent=( 2 if trans.debug else 0 ) ) } <%def name="config_plausible_analytics(plausible_server, plausible_domain)"> %if plausible_server and plausible_domain: - + %else: