From 37846b2d279a102f9446baeac438d1c2212a8a03 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 7 Feb 2022 11:32:00 +0100 Subject: [PATCH 1/4] Make __link_file_check more robust There's no need to load the job and job parameters if the tool isn't upload1. This should fix https://github.com/galaxyproject/galaxy/issues/13311 --- lib/galaxy/jobs/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index 82614ed7b42..bf0f8a42205 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -2258,10 +2258,10 @@ class JobWrapper(HasResourceParameters): method should be removed ASAP and replaced with some properly generic and stateful way of determining link-only datasets. -nate """ - if self.tool: + if self.tool and self.tool.id == 'upload1': job = self.get_job() param_dict = job.get_param_values(self.app) - return self.tool.id == 'upload1' and param_dict.get('link_data_only', None) == 'link_to_files' + return param_dict.get('link_data_only') == 'link_to_files' else: # The tool is unavailable, we try to move the outputs. return False From 83c0683e86733b9b4bdafd6184f50af2c82b0408 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 7 Feb 2022 11:12:54 +0100 Subject: [PATCH 2/4] Restore Metadata size limit Broke in https://github.com/galaxyproject/galaxy/pull/11902 --- lib/galaxy/model/mapping.py | 7 +++-- test/unit/data/test_metadata_limit.py | 44 +++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 test/unit/data/test_metadata_limit.py diff --git a/lib/galaxy/model/mapping.py b/lib/galaxy/model/mapping.py index c77fc982d4b..ceb2623774c 100644 --- a/lib/galaxy/model/mapping.py +++ b/lib/galaxy/model/mapping.py @@ -43,6 +43,7 @@ from galaxy import model from galaxy.model.base import SharedModelMapping from galaxy.model.custom_types import ( JSONType, + MetadataType, MutableJSONType, TrimmedString, UUIDType, @@ -248,7 +249,7 @@ model.HistoryDatasetAssociation.table = Table( Column("peek", TEXT, key="_peek"), Column("tool_version", TEXT), Column("extension", TrimmedString(64)), - Column("metadata", JSONType, key="_metadata"), + Column("metadata", MetadataType, key="_metadata"), Column("parent_id", Integer, ForeignKey("history_dataset_association.id"), nullable=True), Column("designation", TrimmedString(255)), Column("deleted", Boolean, index=True, default=False), @@ -271,7 +272,7 @@ model.HistoryDatasetAssociationHistory.table = Table( Column("version", Integer), Column("name", TrimmedString(255)), Column("extension", TrimmedString(64)), - Column("metadata", JSONType, key="_metadata"), + Column("metadata", MetadataType, key="_metadata"), Column("extended_metadata_id", Integer, ForeignKey("extended_metadata.id"), index=True), ) @@ -527,7 +528,7 @@ model.LibraryDatasetDatasetAssociation.table = Table( Column("peek", TEXT, key="_peek"), Column("tool_version", TEXT), Column("extension", TrimmedString(64)), - Column("metadata", JSONType, key="_metadata"), + Column("metadata", MetadataType, key="_metadata"), Column("parent_id", Integer, ForeignKey("library_dataset_dataset_association.id"), nullable=True), Column("designation", TrimmedString(255)), Column("deleted", Boolean, index=True, default=False), diff --git a/test/unit/data/test_metadata_limit.py b/test/unit/data/test_metadata_limit.py new file mode 100644 index 00000000000..e0c2121a2bc --- /dev/null +++ b/test/unit/data/test_metadata_limit.py @@ -0,0 +1,44 @@ +import pytest + +import galaxy.datatypes.registry as registry +import galaxy.model.mapping as mapping +from galaxy.model import ( + custom_types, + HistoryDatasetAssociation, + set_datatypes_registry, +) + +METADATA_LIMIT = 500 + + +@pytest.fixture(scope="module") +def datatypes_registry(): + r = registry.Registry() + r.load_datatypes() + set_datatypes_registry(r) + + +@pytest.fixture +def sa_session(datatypes_registry): + custom_types.MAX_METADATA_VALUE_SIZE = METADATA_LIMIT + return mapping.init("/tmp", "sqlite:///:memory:", create_tables=True).session + + +def create_bed_data(sa_session, string_size): + hda = HistoryDatasetAssociation(extension="bed") + big_string = "0" * string_size + sa_session.add(hda) + hda.metadata.column_names = [big_string] + assert hda.metadata.column_names + sa_session.flush() + return hda + + +def test_hda_below_limit(sa_session): + hda = create_bed_data(sa_session=sa_session, string_size=1) + assert len(hda.metadata.column_names[0]) == 1 + + +def test_hda_above_limit(sa_session): + hda = create_bed_data(sa_session=sa_session, string_size=1000) + assert not hda.metadata.column_names From bec9d4b49f4fec037b7f4ec46cea3dd36d15e12a Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 8 Feb 2022 12:36:28 +0100 Subject: [PATCH 3/4] Fix resuming job when job has optional data parameters This fixes ``` ERROR galaxy.tools.actions:__init__.py:683 Cannot remap rerun dependencies. Traceback (most recent call last): File "/Users/mvandenb/src/galaxy/lib/galaxy/tools/actions/__init__.py", line 664, in _remap_job_on_rerun self.__remap_parameters(job_to_remap, jtid, jtod, out_data) File "/Users/mvandenb/src/galaxy/lib/galaxy/tools/actions/__init__.py", line 694, in __remap_parameters input_values = {p.name: json.loads(p.value) for p in job_to_remap.parameters} File "/Users/mvandenb/src/galaxy/lib/galaxy/tools/actions/__init__.py", line 694, in input_values = {p.name: json.loads(p.value) for p in job_to_remap.parameters} File "/usr/local/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/json/__init__.py", line 339, in loads raise TypeError(f'the JSON object must be str, bytes or bytearray, ' TypeError: the JSON object must be str, bytes or bytearray, not NoneType ``` Optional data inputs or optional selects are stored as `None` (super inconsistent, since most other parameters are stored as JOSN. We should create "basic_2.py" using pydantic at one point not too far into the future ...). This means we can't call `json.loads` on these. Fortunately this is the only place we do it, and we don't need to consider optional parameters here anyway. --- lib/galaxy/tools/actions/__init__.py | 5 +++-- lib/galaxy_test/api/test_workflows.py | 1 + test/functional/tools/identifier_multiple_in_conditional.xml | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tools/actions/__init__.py b/lib/galaxy/tools/actions/__init__.py index d622df5324d..9080b4fa798 100644 --- a/lib/galaxy/tools/actions/__init__.py +++ b/lib/galaxy/tools/actions/__init__.py @@ -675,12 +675,13 @@ class DefaultToolAction: return remapped_hdas def __remap_parameters(self, job_to_remap, jtid, jtod, out_data): - input_values = {p.name: json.loads(p.value) for p in job_to_remap.parameters} + input_values = {p.name: json.loads(p.value) for p in job_to_remap.parameters if p.value is not None} old_dataset_id = jtod.dataset_id new_dataset_id = out_data[jtod.name].id input_values = update_dataset_ids(input_values, {old_dataset_id: new_dataset_id}, src='hda') for p in job_to_remap.parameters: - p.value = json.dumps(input_values[p.name]) + if p.name in input_values: + p.value = json.dumps(input_values[p.name]) jtid.dataset = out_data[jtod.name] jtid.dataset.hid = jtod.dataset.hid log.info(f'Job {job_to_remap.id} input HDA {jtod.dataset.id} remapped to new HDA {jtid.dataset.id}') diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index ae0b636aecf..6f26a060ca4 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -967,6 +967,7 @@ steps: cond_param_inner: true input1: $link: 0/out_file1 + thedata: null cat: tool_id: cat1 in: diff --git a/test/functional/tools/identifier_multiple_in_conditional.xml b/test/functional/tools/identifier_multiple_in_conditional.xml index 81d12fd99fa..8bff2256f0b 100644 --- a/test/functional/tools/identifier_multiple_in_conditional.xml +++ b/test/functional/tools/identifier_multiple_in_conditional.xml @@ -16,6 +16,7 @@ + From 9cb0c45ca4c1dea8667ee5ddd6dc3a2460fc0b8e Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 8 Feb 2022 09:57:50 -0500 Subject: [PATCH 4/4] Add .git-blame-ignore-revs to 21.09. Without this, if you have set the file up in git config blame will blow up with something like `could not open object name list: .git-blame-ignore-revs`. I only included the relevant hashes here, will need to handle a merge as it goes forward. --- .git-blame-ignore-revs | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .git-blame-ignore-revs diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs new file mode 100644 index 00000000000..29a799793ca --- /dev/null +++ b/.git-blame-ignore-revs @@ -0,0 +1,3 @@ +# Migrate code style to Prettier +5b2928f851bd5ea3b9c2a04abf2cee9ff0bc54cc +87873c5e2f4e6b97fe0f2084bfca0295fcd471de