From f21cbdaa9fd77757d09569d828ba5a34ebec07cf Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 3 Apr 2024 11:06:29 +0200 Subject: [PATCH 1/8] Always serialize element_count and populated when listing collection contents Fixes https://github.com/galaxyproject/galaxy/issues/17883 --- lib/galaxy/managers/collections_util.py | 23 ++++++++++++------- lib/galaxy/model/__init__.py | 6 ++++- lib/galaxy/tools/__init__.py | 23 ++++++++++++------- .../api/test_dataset_collections.py | 4 +++- 4 files changed, 38 insertions(+), 18 deletions(-) diff --git a/lib/galaxy/managers/collections_util.py b/lib/galaxy/managers/collections_util.py index f5af1c1eeb63..0896c2f82dfc 100644 --- a/lib/galaxy/managers/collections_util.py +++ b/lib/galaxy/managers/collections_util.py @@ -1,5 +1,9 @@ import logging import math +from typing import ( + Any, + Dict, +) from galaxy import ( exceptions, @@ -153,7 +157,9 @@ def dictify_dataset_collection_instance( return dict_value -def dictify_element_reference(element, rank_fuzzy_counts=None, recursive=True, security=None): +def dictify_element_reference( + element: model.DatasetCollectionElement, rank_fuzzy_counts=None, recursive=True, security=None +): """Load minimal details of elements required to show outline of contents in history panel. History panel can use this reference to expand to full details if individual dataset elements @@ -161,27 +167,28 @@ def dictify_element_reference(element, rank_fuzzy_counts=None, recursive=True, s """ dictified = element.to_dict(view="element") if (element_object := element.element_object) is not None: - object_details = dict( + object_details: Dict[str, Any] = dict( id=element_object.id, model_class=element_object.__class__.__name__, ) - if element.child_collection: + if isinstance(element_object, model.DatasetCollection): object_details["collection_type"] = element_object.collection_type + object_details["element_count"] = element_object.element_count + object_details["populated"] = element_object.populated_optimized # Recursively yield elements for each nested collection... if recursive: - child_collection = element.child_collection - elements, rest_fuzzy_counts = get_fuzzy_count_elements(child_collection, rank_fuzzy_counts) + elements, rest_fuzzy_counts = get_fuzzy_count_elements(element_object, rank_fuzzy_counts) object_details["elements"] = [ dictify_element_reference(_, rank_fuzzy_counts=rest_fuzzy_counts, recursive=recursive) for _ in elements ] - object_details["element_count"] = child_collection.element_count else: object_details["state"] = element_object.state object_details["hda_ldda"] = "hda" - object_details["history_id"] = element_object.history_id - object_details["tags"] = element_object.make_tag_string_list() + if isinstance(element_object, model.HistoryDatasetAssociation): + object_details["history_id"] = element_object.history_id + object_details["tags"] = element_object.make_tag_string_list() dictified["object"] = object_details else: diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index b1f6f4ca123c..513362112795 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -4996,6 +4996,8 @@ class HistoryDatasetAssociation(DatasetInstance, HasTags, Dictifiable, UsesAnnot Resource class that creates a relation between a dataset and a user history. """ + history_id: Optional[int] + def __init__( self, hid=None, @@ -7153,7 +7155,9 @@ def is_collection(self): return self.element_type == "dataset_collection" @property - def element_object(self): + def element_object( + self, + ) -> Optional[Union[HistoryDatasetAssociation, LibraryDatasetDatasetAssociation, DatasetCollection]]: if self.hda: return self.hda elif self.ldda: diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index d50725c4a04d..38e00c4d0748 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -3461,7 +3461,9 @@ def _get_new_elements(self, history, elements_to_copy): @staticmethod def element_is_valid(element: model.DatasetCollectionElement): - return element.element_object.is_ok + element_object = element.element_object + assert isinstance(element_object, model.DatasetInstance) + return element_object.is_ok def produce_outputs(self, trans, out_data, output_collections, incoming, history, **kwds): collection = incoming["input"] @@ -3503,7 +3505,9 @@ class FilterFailedDatasetsTool(FilterDatasetsTool): @staticmethod def element_is_valid(element: model.DatasetCollectionElement): - return element.element_object.is_ok + element_object = element.element_object + assert isinstance(element_object, model.DatasetInstance) + return element_object.is_ok class KeepSuccessDatasetsTool(FilterDatasetsTool): @@ -3514,12 +3518,14 @@ class KeepSuccessDatasetsTool(FilterDatasetsTool): @staticmethod def element_is_valid(element: model.DatasetCollectionElement): + element_object = element.element_object + assert isinstance(element_object, model.DatasetInstance) if ( - element.element_object.state != model.Dataset.states.PAUSED - and element.element_object.state in model.Dataset.non_ready_states + element_object.state != model.Dataset.states.PAUSED + and element_object.state in model.Dataset.non_ready_states ): raise ToolInputsNotReadyException("An input dataset is pending.") - return element.element_object.is_ok + return element_object.is_ok class FilterEmptyDatasetsTool(FilterDatasetsTool): @@ -3528,10 +3534,11 @@ class FilterEmptyDatasetsTool(FilterDatasetsTool): @staticmethod def element_is_valid(element: model.DatasetCollectionElement): - dataset_instance: model.DatasetInstance = element.element_object - if dataset_instance.has_data(): + element_object = element.element_object + assert isinstance(element_object, model.DatasetInstance) + if element_object.has_data(): # We have data, but it might just be a compressed archive of nothing - file_name = dataset_instance.get_file_name() + file_name = element_object.get_file_name() _, fh = get_fileobj_raw(file_name, mode="rb") if len(fh.read(1)): return True diff --git a/lib/galaxy_test/api/test_dataset_collections.py b/lib/galaxy_test/api/test_dataset_collections.py index f24f8f70786b..85b0652c0259 100644 --- a/lib/galaxy_test/api/test_dataset_collections.py +++ b/lib/galaxy_test/api/test_dataset_collections.py @@ -455,7 +455,7 @@ def test_show_dataset_collection_contents(self, history_id): # Get contents_url from history contents, use it to show the first level # of collection contents in the created HDCA, then use it again to drill # down into the nested collection contents - hdca = self.dataset_collection_populator.create_list_of_list_in_history(history_id).json() + hdca = self.dataset_collection_populator.create_list_of_list_in_history(history_id, wait=True).json() root_contents_url = self._get_contents_url_for_hdca(history_id, hdca) # check root contents for this collection @@ -466,6 +466,8 @@ def test_show_dataset_collection_contents(self, history_id): # drill down, retrieve nested collection contents assert "object" in root_contents[0] assert "contents_url" in root_contents[0]["object"] + assert root_contents[0]["object"]["element_count"] == 3 + assert root_contents[0]["object"]["populated"] drill_contents_url = root_contents[0]["object"]["contents_url"] drill_contents = self._get(drill_contents_url).json() assert len(drill_contents) == len(hdca["elements"][0]["object"]["elements"]) From 8ab16a9f6f4455950df58a4192b0d1f5d32cc165 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 3 Apr 2024 13:23:36 +0200 Subject: [PATCH 2/8] Make wait_for_history_jobs look at jobs, not datasets That fixes test_optional_workflow_output, which started failing after https://github.com/galaxyproject/galaxy/pull/17874, which removed the last static output of `output_filter`, and so there would never be any active datasets to wait on. --- lib/galaxy_test/base/populators.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy_test/base/populators.py b/lib/galaxy_test/base/populators.py index c4f7887e14ac..ce26be0a4c41 100644 --- a/lib/galaxy_test/base/populators.py +++ b/lib/galaxy_test/base/populators.py @@ -630,7 +630,8 @@ def has_active_jobs(): raise TimeoutAssertionError(message) if assert_ok: - self.wait_for_history(history_id, assert_ok=True, timeout=timeout) + for job in self.history_jobs(history_id=history_id): + assert job["state"] in ("ok", "skipped"), f"Job {job} not in expected state" def wait_for_jobs( self, From 1332eb162006b09a5a03fd8f371cd7458e2bf128 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 3 Apr 2024 13:10:53 +0100 Subject: [PATCH 3/8] Fix ``test_workflow_optional_input_filtering`` API test to produce one output again --- lib/galaxy_test/api/test_workflows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index f8e1cb85ca6e..3c91c601b523 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -5477,7 +5477,7 @@ def test_optional_workflow_output(self): tool_id: output_filter state: produce_out_1: False - filter_text_1: '1' + filter_text_1: 'foo' produce_collection: False """, test_data={}, From 4984523c39f89380315db46a174791a01769575c Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 3 Apr 2024 13:11:33 +0100 Subject: [PATCH 4/8] Change test with no outputs to not fail ``out_3`` has been removed in https://github.com/galaxyproject/galaxy/pull/17874 . --- test/functional/tools/output_filter.xml | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/functional/tools/output_filter.xml b/test/functional/tools/output_filter.xml index 4a891399210d..584f2f43cf3f 100644 --- a/test/functional/tools/output_filter.xml +++ b/test/functional/tools/output_filter.xml @@ -59,15 +59,9 @@ echo 'p2.reverse' > p2.reverse - - + - - - - - From 16e0e4b775541a8bcc40a699eb302dfdbc945790 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 3 Apr 2024 18:46:39 +0200 Subject: [PATCH 5/8] Only update job if state really changed --- lib/galaxy/model/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index b1f6f4ca123c..a8747f62bddd 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -1694,7 +1694,7 @@ def set_state(self, state: JobState) -> bool: # generate statement that will not revert DELETING or DELETED back to anything non-terminal rval = session.execute( update(Job.table) - .where(Job.id == self.id, ~Job.state.in_((Job.states.DELETING, Job.states.DELETED))) + .where(Job.id == self.id, ~Job.state.in_((state, *Job.finished_states))) .values(state=state) ) if rval.rowcount == 1: From 061d4c32db3cab5a880bad912deaa89c06acb641 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 3 Apr 2024 19:02:19 +0200 Subject: [PATCH 6/8] Commit after changing job state --- lib/galaxy/model/__init__.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index a8747f62bddd..795e1dbddb67 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -1697,9 +1697,10 @@ def set_state(self, state: JobState) -> bool: .where(Job.id == self.id, ~Job.state.in_((state, *Job.finished_states))) .values(state=state) ) + with transaction(session): + session.commit() if rval.rowcount == 1: # Need to expire state since we just updated it, but ORM doesn't know about it. - session.expire(self, ["state"]) self.state_history.append(JobStateHistory(self)) return True else: From 2594276b13472b9ea9c2332103bbfab3281f1265 Mon Sep 17 00:00:00 2001 From: Marius van den Beek Date: Wed, 3 Apr 2024 19:39:50 +0200 Subject: [PATCH 7/8] Update comments Co-authored-by: John Davis --- lib/galaxy/model/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 795e1dbddb67..8097776a56b5 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -1691,7 +1691,7 @@ def set_state(self, state: JobState) -> bool: return False session = object_session(self) if session and self.id and state not in Job.finished_states: - # generate statement that will not revert DELETING or DELETED back to anything non-terminal + # Do not update if job is in a terminal state rval = session.execute( update(Job.table) .where(Job.id == self.id, ~Job.state.in_((state, *Job.finished_states))) @@ -1700,7 +1700,6 @@ def set_state(self, state: JobState) -> bool: with transaction(session): session.commit() if rval.rowcount == 1: - # Need to expire state since we just updated it, but ORM doesn't know about it. self.state_history.append(JobStateHistory(self)) return True else: From ed5b0023ff4bcbafe3a0cde461eb7b4feb59d721 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Thu, 4 Apr 2024 02:32:32 +0100 Subject: [PATCH 8/8] Fix search and version menu in docs The `html_theme_path` option should not be used any more with sphinx-rtd-theme since it switches off the theme's ability to automatically enable the sphinxcontrib-jquery extension, i.e. jQuery is not loaded in the web pages when building the docs with Sphinx >=6. xref: https://github.com/readthedocs/sphinx_rtd_theme/issues/1434#issuecomment-1472671651 Also replace the deprecated `canonical_url` theme option with Sphinx's `html_baseurl`. xref: https://sphinx-rtd-theme.readthedocs.io/en/stable/configuring.html#confval-canonical_url --- doc/source/conf.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/doc/source/conf.py b/doc/source/conf.py index 47730a4f5327..284d7b93242b 100644 --- a/doc/source/conf.py +++ b/doc/source/conf.py @@ -14,8 +14,6 @@ import os import sys -import sphinx_rtd_theme - # Set GALAXY_DOCS_SKIP_VIEW_CODE=1 to skip embedding highlighted source # code into docs. SKIP_VIEW_CODE = os.environ.get("GALAXY_DOCS_SKIP_VIEW_CODE", False) == "1" @@ -161,11 +159,10 @@ def setup(app): "collapse_navigation": False, "display_version": True, "navigation_depth": 2, - "canonical_url": "https://docs.galaxyproject.org/en/master/", } # Add any paths that contain custom themes here, relative to this directory. -html_theme_path = [sphinx_rtd_theme.get_html_theme_path()] +# html_theme_path = [] # The name for this set of Sphinx documents. If None, it defaults to # " v documentation". @@ -174,6 +171,8 @@ def setup(app): # A shorter title for the navigation bar. Default is the same as html_title. # html_short_title = None +html_baseurl = "https://docs.galaxyproject.org/en/master/" + # The name of an image file (relative to this directory) to place at the top # of the sidebar. # html_logo = None