From dd171fe34df25a7fdc206b9738a008c1dc2defca Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 13 Jun 2024 11:50:37 +0200 Subject: [PATCH 01/10] Add test tool for default ext handling in discovered collections --- .../functional/tools/discover_default_ext.xml | 52 +++++++++++++++++++ test/functional/tools/sample_tool_conf.xml | 1 + 2 files changed, 53 insertions(+) create mode 100644 test/functional/tools/discover_default_ext.xml diff --git a/test/functional/tools/discover_default_ext.xml b/test/functional/tools/discover_default_ext.xml new file mode 100644 index 000000000000..7f7c7d83cdd8 --- /dev/null +++ b/test/functional/tools/discover_default_ext.xml @@ -0,0 +1,52 @@ + + 1.txt; +]]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/sample_tool_conf.xml b/test/functional/tools/sample_tool_conf.xml index 2c074381db5e..a9277f4fab78 100644 --- a/test/functional/tools/sample_tool_conf.xml +++ b/test/functional/tools/sample_tool_conf.xml @@ -206,6 +206,7 @@ + From e5fca086712051d04568fb028be15c2901e86184 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 13 Jun 2024 12:15:00 +0200 Subject: [PATCH 02/10] Assign default `data` extension on discovered collection output They used to be persisted as null in the database prior to this change. --- lib/galaxy/model/store/discover.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/model/store/discover.py b/lib/galaxy/model/store/discover.py index 679bd2aa38f7..17389c7b83eb 100644 --- a/lib/galaxy/model/store/discover.py +++ b/lib/galaxy/model/store/discover.py @@ -38,6 +38,10 @@ from galaxy.util.hash_util import HASH_NAME_MAP if TYPE_CHECKING: + from galaxy.job_execution.output_collect import ( + DatasetCollector, + ToolMetadataDatasetCollector, + ) from galaxy.model.store import ModelExportStore log = logging.getLogger(__name__) @@ -50,7 +54,7 @@ class MaxDiscoveredFilesExceededError(ValueError): pass -CollectorT = Any # TODO: setup an interface for these file collectors data classes. +CollectorT = Union["DatasetCollector", "ToolMetadataDatasetCollector"] class ModelPersistenceContext(metaclass=abc.ABCMeta): @@ -1056,19 +1060,21 @@ def name(self): return self.as_dict.get("name") @property - def dbkey(self): - return self.as_dict.get("dbkey", getattr(self.collector, "default_dbkey", "?")) + def dbkey(self) -> str: + return self.as_dict.get("dbkey", self.collector and self.collector.default_dbkey or "?") @property - def ext(self): - return self.as_dict.get("ext", getattr(self.collector, "default_ext", "data")) + def ext(self) -> str: + return self.as_dict.get("ext", self.collector and self.collector.default_ext or "data") @property - def visible(self): + def visible(self) -> bool: try: return self.as_dict["visible"].lower() == "visible" except KeyError: - return getattr(self.collector, "default_visible", True) + if self.collector and self.collector.default_visible is not None: + return self.collector.default_visible + return True @property def link_data(self): From 6551f31671b51e664ca4bd2b0c1dd3e716922d17 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 13 Jun 2024 12:52:48 +0200 Subject: [PATCH 03/10] Implement default format on collection level --- .../tool_util/parser/output_collection_def.py | 8 +++++++- lib/galaxy/tool_util/xsd/galaxy.xsd | 10 ++++++---- .../app/tools/test_collect_primary_datasets.py | 14 ++++++++++++++ 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/tool_util/parser/output_collection_def.py b/lib/galaxy/tool_util/parser/output_collection_def.py index 4d16cb5d3539..b7929ebe1665 100644 --- a/lib/galaxy/tool_util/parser/output_collection_def.py +++ b/lib/galaxy/tool_util/parser/output_collection_def.py @@ -34,7 +34,13 @@ def dataset_collector_descriptions_from_elem(elem, legacy=True): if num_discover_dataset_blocks == 0 and legacy: collectors = [DEFAULT_DATASET_COLLECTOR_DESCRIPTION] else: - collectors = [dataset_collection_description(**e.attrib) for e in primary_dataset_elems] + default_format = elem.attrib.get("format") + collectors = [] + for e in primary_dataset_elems: + description_attributes = e.attrib + if default_format and "format" not in description_attributes and "ext" not in description_attributes: + description_attributes["format"] = default_format + collectors.append(dataset_collection_description(**description_attributes)) return _validate_collectors(collectors) diff --git a/lib/galaxy/tool_util/xsd/galaxy.xsd b/lib/galaxy/tool_util/xsd/galaxy.xsd index bc1f18780a60..daebc29070ac 100644 --- a/lib/galaxy/tool_util/xsd/galaxy.xsd +++ b/lib/galaxy/tool_util/xsd/galaxy.xsd @@ -5360,11 +5360,13 @@ The default is ``galaxy.json``. - The short name for the output datatype. -The valid values for format can be found in + +(e.g. ``format="pdf"`` or ``format="fastqsanger"``). For collections this is the default +format for all included elements. Note that the format specified here is ignored for +discovered data sets on Galaxy versions prior to 24.0 and should be specified using the ```` tag set. + ]]> diff --git a/test/unit/app/tools/test_collect_primary_datasets.py b/test/unit/app/tools/test_collect_primary_datasets.py index 16f3c6aae13e..fb612853b4db 100644 --- a/test/unit/app/tools/test_collect_primary_datasets.py +++ b/test/unit/app/tools/test_collect_primary_datasets.py @@ -128,6 +128,20 @@ def test_collect_multiple_recurse_dict(self): created_hda_3 = datasets[DEFAULT_TOOL_OUTPUT]["test3"] assert_created_with_path(self.app.object_store, created_hda_3.dataset, path3) + def test_collect_collection_default_format(self): + self._replace_output_collectors( + """ + + """ + ) + self._setup_extra_file(subdir="subdir_for_name_discovery", filename="test1") + self._setup_extra_file(subdir="subdir_for_name_discovery", filename="test2") + + datasets = self._collect() + assert DEFAULT_TOOL_OUTPUT in datasets + for dataset in datasets[DEFAULT_TOOL_OUTPUT].values(): + assert dataset.ext == "abcdef" + def test_collect_sorted_reverse(self): self._replace_output_collectors( """ From 55439e455e81b7e900b21d4926495ac4abb58ac9 Mon Sep 17 00:00:00 2001 From: Alireza Heidari Date: Thu, 13 Jun 2024 21:17:57 +0200 Subject: [PATCH 04/10] =?UTF-8?q?=F0=9F=90=9B:=20fix=20workflow=20tags=20i?= =?UTF-8?q?nitialization=20in=20`Workflow/Editor/Index`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- client/src/components/Workflow/Editor/Index.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/components/Workflow/Editor/Index.vue b/client/src/components/Workflow/Editor/Index.vue index ffb07080dc1f..66bd47b66e27 100644 --- a/client/src/components/Workflow/Editor/Index.vue +++ b/client/src/components/Workflow/Editor/Index.vue @@ -354,7 +354,7 @@ export default { } } - const tags = ref([]); + const tags = ref(props.workflowTags); const setTagsHandler = new SetValueActionHandler( undoRedoStore, (value) => (tags.value = structuredClone(value)), From d3d06d4e87a7d63291820e0ca59a2d712cd16b4d Mon Sep 17 00:00:00 2001 From: Alireza Heidari Date: Fri, 14 Jun 2024 11:44:34 +0200 Subject: [PATCH 05/10] =?UTF-8?q?=F0=9F=9B=A0=EF=B8=8F:=20spread=20`props.?= =?UTF-8?q?workflowTags`=20in=20`Workflow/Editor/Index`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Marius van den Beek --- client/src/components/Workflow/Editor/Index.vue | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/components/Workflow/Editor/Index.vue b/client/src/components/Workflow/Editor/Index.vue index 66bd47b66e27..1b582dd77f6a 100644 --- a/client/src/components/Workflow/Editor/Index.vue +++ b/client/src/components/Workflow/Editor/Index.vue @@ -354,7 +354,7 @@ export default { } } - const tags = ref(props.workflowTags); + const tags = ref([...props.workflowTags]); const setTagsHandler = new SetValueActionHandler( undoRedoStore, (value) => (tags.value = structuredClone(value)), From 325091bb5335c433e8a7776e39c56ddfd17d3f0f Mon Sep 17 00:00:00 2001 From: Alireza Heidari Date: Tue, 18 Jun 2024 16:47:45 +0200 Subject: [PATCH 06/10] =?UTF-8?q?=F0=9F=9B=A0=EF=B8=8F:=20initiate=20`tags?= =?UTF-8?q?`=20ref=20using=20immediate=20watch=20for=20`props.workflowTags?= =?UTF-8?q?`=20in=20`Workflow/Editor/Index`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- client/src/components/Workflow/Editor/Index.vue | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/client/src/components/Workflow/Editor/Index.vue b/client/src/components/Workflow/Editor/Index.vue index 1b582dd77f6a..84904cecc855 100644 --- a/client/src/components/Workflow/Editor/Index.vue +++ b/client/src/components/Workflow/Editor/Index.vue @@ -184,7 +184,7 @@ import { useMagicKeys, whenever } from "@vueuse/core"; import { logicAnd, logicNot, logicOr } from "@vueuse/math"; import { Toast } from "composables/toast"; import { storeToRefs } from "pinia"; -import Vue, { computed, nextTick, onUnmounted, ref, unref } from "vue"; +import Vue, { computed, nextTick, onUnmounted, ref, unref, watch } from "vue"; import { getUntypedWorkflowParameters } from "@/components/Workflow/Editor/modules/parameters"; import { ConfirmDialog } from "@/composables/confirmDialog"; @@ -354,7 +354,16 @@ export default { } } - const tags = ref([...props.workflowTags]); + const tags = ref([]); + + watch( + () => props.workflowTags, + (newTags) => { + tags.value = [...newTags]; + }, + { immediate: true } + ); + const setTagsHandler = new SetValueActionHandler( undoRedoStore, (value) => (tags.value = structuredClone(value)), From f7edaabfaa888c97f618286da45b7c72297d5393 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Fri, 14 Jun 2024 15:57:14 -0500 Subject: [PATCH 07/10] [24.1] Fix `input_step_parameters` missing vals without label `input_step_parameters` that did not have labels (label=None) were being excluded in the `WorkflowInvocation.to_dict()` result. This therefore sets the label to `n: Unnamed parameter` for null labels. Fixes https://github.com/galaxyproject/galaxy/issues/18366 --- .../WorkflowInvocationState/WorkflowInvocationStep.vue | 7 ++++++- lib/galaxy/model/__init__.py | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/client/src/components/WorkflowInvocationState/WorkflowInvocationStep.vue b/client/src/components/WorkflowInvocationState/WorkflowInvocationStep.vue index 4678418e6f5d..243c44d428d1 100644 --- a/client/src/components/WorkflowInvocationState/WorkflowInvocationStep.vue +++ b/client/src/components/WorkflowInvocationState/WorkflowInvocationStep.vue @@ -74,7 +74,7 @@ + :parameters="[getParamInput(stepDetails)]" /> Date: Fri, 14 Jun 2024 16:08:44 -0500 Subject: [PATCH 08/10] cast invocation state query route prop `fromPanel` to Boolean --- client/src/entry/analysis/router.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/src/entry/analysis/router.js b/client/src/entry/analysis/router.js index 16484c23f5d7..71b66c66ed9a 100644 --- a/client/src/entry/analysis/router.js +++ b/client/src/entry/analysis/router.js @@ -628,7 +628,7 @@ export function getRouter(Galaxy) { props: (route) => ({ invocationId: route.params.invocationId, isFullPage: true, - fromPanel: route.query.from_panel, + fromPanel: Boolean(route.query.from_panel), }), }, { From b5cabbfc08a9a5fbe7da5a6d1dde85dbe44b64c4 Mon Sep 17 00:00:00 2001 From: Ahmed Awan Date: Fri, 14 Jun 2024 16:09:17 -0500 Subject: [PATCH 09/10] ensure correct version routed to on workflow run This is a bug caused in https://github.com/galaxyproject/galaxy/pull/18378 which made the `SaveChangesModal` useless because the version before the save was routed to. Now, we instead append the version to the route after the user confirms the save (or doesn't) in the modal. --- client/src/components/Workflow/Editor/Index.vue | 10 +++++----- .../components/Workflow/Editor/SaveChangesModal.vue | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/client/src/components/Workflow/Editor/Index.vue b/client/src/components/Workflow/Editor/Index.vue index 57a96b367335..71983b2421d0 100644 --- a/client/src/components/Workflow/Editor/Index.vue +++ b/client/src/components/Workflow/Editor/Index.vue @@ -801,12 +801,9 @@ export default { this.report.markdown = markdown; }, onRun() { - const runUrl = `/workflows/run?id=${this.id}${ - this.version !== undefined ? `&version=${this.version}` : "" - }`; - this.onNavigate(runUrl); + this.onNavigate(`/workflows/run?id=${this.id}`, false, false, true); }, - async onNavigate(url, forceSave = false, ignoreChanges = false) { + async onNavigate(url, forceSave = false, ignoreChanges = false, appendVersion = false) { if (this.isNewTempWorkflow) { await this.onCreate(); } else if (this.hasChanges && !forceSave && !ignoreChanges) { @@ -819,6 +816,9 @@ export default { await this.onSave(); } + if (appendVersion && this.version !== undefined) { + url += `&version=${this.version}`; + } this.hasChanges = false; await nextTick(); this.$router.push(url); diff --git a/client/src/components/Workflow/Editor/SaveChangesModal.vue b/client/src/components/Workflow/Editor/SaveChangesModal.vue index 9a2b5bbda2d0..921fa76a4b9e 100644 --- a/client/src/components/Workflow/Editor/SaveChangesModal.vue +++ b/client/src/components/Workflow/Editor/SaveChangesModal.vue @@ -24,7 +24,7 @@ const busy = ref(false); const emit = defineEmits<{ /** Proceed with or without saving the changes */ - (e: "on-proceed", url: string, forceSave: boolean, ignoreChanges: boolean): void; + (e: "on-proceed", url: string, forceSave: boolean, ignoreChanges: boolean, appendVersion: boolean): void; /** Update the nav URL prop */ (e: "update:nav-url", url: string): void; /** Update the show modal boolean prop */ @@ -49,13 +49,13 @@ function closeModal() { function dontSave() { busy.value = true; - emit("on-proceed", props.navUrl, false, true); + emit("on-proceed", props.navUrl, false, true, true); } function saveChanges() { busy.value = true; closeModal(); - emit("on-proceed", props.navUrl, true, false); + emit("on-proceed", props.navUrl, true, false, true); } From 111d5632ff4bec34485615fa07693466828f0450 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 19 Jun 2024 10:24:55 +0200 Subject: [PATCH 10/10] Run make format --- test/unit/app/tools/test_parameter_validation.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/unit/app/tools/test_parameter_validation.py b/test/unit/app/tools/test_parameter_validation.py index 23df864ae3e4..e143b4feed7c 100644 --- a/test/unit/app/tools/test_parameter_validation.py +++ b/test/unit/app/tools/test_parameter_validation.py @@ -217,11 +217,13 @@ def test_InRangeValidator(self): ) p.validate(10) with self.assertRaisesRegex( - ValueError, r"Parameter blah: Value \('15'\) must not fulfill float\('10'\) < float\(value\) <= float\('20'\)" + ValueError, + r"Parameter blah: Value \('15'\) must not fulfill float\('10'\) < float\(value\) <= float\('20'\)", ): p.validate(15) with self.assertRaisesRegex( - ValueError, r"Parameter blah: Value \('20'\) must not fulfill float\('10'\) < float\(value\) <= float\('20'\)" + ValueError, + r"Parameter blah: Value \('20'\) must not fulfill float\('10'\) < float\(value\) <= float\('20'\)", ): p.validate(20) p.validate(21)