From 5cf9f0e4ea1f87209bfee16e6e3da307c102612b Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 22 Nov 2023 15:37:46 +0100 Subject: [PATCH 1/2] Fix extra files collection for outputs in ``store_by="id"`` object stores --- lib/galaxy/job_execution/output_collect.py | 10 +++++++--- lib/galaxy/jobs/__init__.py | 2 +- test/integration/test_legacy_store_by.py | 21 +++++++++++++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 test/integration/test_legacy_store_by.py diff --git a/lib/galaxy/job_execution/output_collect.py b/lib/galaxy/job_execution/output_collect.py index 9464de383e9..f8a94fb2f39 100644 --- a/lib/galaxy/job_execution/output_collect.py +++ b/lib/galaxy/job_execution/output_collect.py @@ -718,9 +718,13 @@ def default_exit_code_file(files_dir, id_tag): return os.path.join(files_dir, f"galaxy_{id_tag}.ec") -def collect_extra_files(object_store, dataset, job_working_directory): +def collect_extra_files(object_store, dataset, job_working_directory, outputs_to_working_directory=False): # TODO: should this use compute_environment to determine the extra files path ? - file_name = dataset.dataset.extra_files_path_name_from(object_store) + real_file_name = file_name = dataset.dataset.extra_files_path_name_from(object_store) + if outputs_to_working_directory: + # OutputsToWorkingDirectoryPathRewriter always rewrites extra files to uuid path, + # so we have to collect from that path even if the real extra files path is dataset_N_files + file_name = f"dataset_{dataset.dataset.uuid}_files" output_location = "outputs" temp_file_path = os.path.join(job_working_directory, output_location, file_name) if not os.path.exists(temp_file_path): @@ -739,7 +743,7 @@ def collect_extra_files(object_store, dataset, job_working_directory): for f in files: object_store.update_from_file( dataset.dataset, - extra_dir=os.path.normpath(os.path.join(file_name, os.path.relpath(root, temp_file_path))), + extra_dir=os.path.normpath(os.path.join(real_file_name, os.path.relpath(root, temp_file_path))), alt_name=f, file_name=os.path.join(root, f), create=True, diff --git a/lib/galaxy/jobs/__init__.py b/lib/galaxy/jobs/__init__.py index 004dd45e2f9..2e57e2dcc10 100644 --- a/lib/galaxy/jobs/__init__.py +++ b/lib/galaxy/jobs/__init__.py @@ -1652,7 +1652,7 @@ class MinimalJobWrapper(HasResourceParameters): dataset.dataset.uuid = context["uuid"] self.__update_output(job, dataset) if not purged: - collect_extra_files(self.object_store, dataset, self.working_directory) + collect_extra_files(self.object_store, dataset, self.working_directory, self.outputs_to_working_directory) if job.states.ERROR == final_job_state: dataset.blurb = "error" if not implicit_collection_jobs: diff --git a/test/integration/test_legacy_store_by.py b/test/integration/test_legacy_store_by.py new file mode 100644 index 00000000000..7ad98192f1c --- /dev/null +++ b/test/integration/test_legacy_store_by.py @@ -0,0 +1,21 @@ +from galaxy_test.driver import integration_util + + +class StoreByIdTestCase(integration_util.IntegrationInstance): + """Describe a Galaxy test instance with embedded pulsar configured.""" + + framework_tool_and_types = True + + @classmethod + def handle_galaxy_config_kwds(cls, config): + config["object_store_store_by"] = "id" + config["outputs_to_working_directory"] = True + + +instance = integration_util.integration_module_instance(StoreByIdTestCase) + +test_tools = integration_util.integration_tool_runner( + [ + "composite_output_tests", + ] +) From 7fe19a7cadb5e5ac4c7b0e008c989f22b3e04d40 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 22 Nov 2023 15:41:21 +0100 Subject: [PATCH 2/2] Merge ``store_by="id"`` tests --- .../test_change_datatype_with_store_by_id.py | 23 ------------------ test/integration/test_legacy_store_by.py | 24 +++++++++++++++++-- 2 files changed, 22 insertions(+), 25 deletions(-) delete mode 100644 test/integration/test_change_datatype_with_store_by_id.py diff --git a/test/integration/test_change_datatype_with_store_by_id.py b/test/integration/test_change_datatype_with_store_by_id.py deleted file mode 100644 index a7c2c0afd3a..00000000000 --- a/test/integration/test_change_datatype_with_store_by_id.py +++ /dev/null @@ -1,23 +0,0 @@ -from galaxy_test.api.test_workflows import ChangeDatatypeTests -from galaxy_test.base.populators import ( - DatasetPopulator, - WorkflowPopulator, -) -from galaxy_test.driver import integration_util - - -class TestChangeDatatypeStoreByIdIntegration(integration_util.IntegrationTestCase, ChangeDatatypeTests): - """Test changing datatype with object_store_store_by: id.""" - - dataset_populator: DatasetPopulator - framework_tool_and_types = True - - @classmethod - def handle_galaxy_config_kwds(cls, config): - config["object_store_store_by"] = "id" - config["retry_metadata_internally"] = False - - def setUp(self): - super().setUp() - self.dataset_populator = DatasetPopulator(self.galaxy_interactor) - self.workflow_populator = WorkflowPopulator(self.galaxy_interactor) diff --git a/test/integration/test_legacy_store_by.py b/test/integration/test_legacy_store_by.py index 7ad98192f1c..aede8c311c9 100644 --- a/test/integration/test_legacy_store_by.py +++ b/test/integration/test_legacy_store_by.py @@ -1,9 +1,29 @@ +from galaxy_test.api.test_workflows import ChangeDatatypeTests +from galaxy_test.base.populators import ( + DatasetPopulator, + WorkflowPopulator, +) from galaxy_test.driver import integration_util -class StoreByIdTestCase(integration_util.IntegrationInstance): - """Describe a Galaxy test instance with embedded pulsar configured.""" +class TestChangeDatatypeStoreByIdIntegration(integration_util.IntegrationTestCase, ChangeDatatypeTests): + """Test changing datatype with object_store_store_by: id.""" + dataset_populator: DatasetPopulator + framework_tool_and_types = True + + @classmethod + def handle_galaxy_config_kwds(cls, config): + config["object_store_store_by"] = "id" + config["retry_metadata_internally"] = False + + def setUp(self): + super().setUp() + self.dataset_populator = DatasetPopulator(self.galaxy_interactor) + self.workflow_populator = WorkflowPopulator(self.galaxy_interactor) + + +class StoreByIdTestCase(integration_util.IntegrationInstance): framework_tool_and_types = True @classmethod