From fbaa58ace7a2c626bce8c9a682eecb1e45377fad Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 18 Apr 2019 13:24:18 -0400 Subject: [PATCH 1/4] Run API tests with a nested object store. --- run_tests.sh | 2 ++ test/base/driver_util.py | 30 ++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/run_tests.sh b/run_tests.sh index 5634c1d7793..0efac9e7111 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -362,6 +362,8 @@ do fi ;; -a|-api|--api) + GALAXY_TEST_USE_HIERARCHICAL_OBJECT_STORE="True" # Run these tests with a non-trivial object store. + export GALAXY_TEST_USE_HIERARCHICAL_OBJECT_STORE GALAXY_TEST_TOOL_CONF="config/tool_conf.xml.sample,test/functional/tools/samples_tool_conf.xml" test_script="pytest" report_file="./run_api_tests.html" diff --git a/test/base/driver_util.py b/test/base/driver_util.py index 7b09c9b91eb..c828c848389 100644 --- a/test/base/driver_util.py +++ b/test/base/driver_util.py @@ -240,6 +240,36 @@ def setup_galaxy_config( ) config.update(database_conf(tmpdir, prefer_template_database=prefer_template_database)) config.update(install_database_conf(tmpdir, default_merged=default_install_db_merged)) + if asbool(os.environ.get("GALAXY_TEST_USE_HIERARCHICAL_OBJECT_STORE")): + object_store_config = os.path.join(tmpdir, "object_store_conf.yml") + with open(object_store_config, "w") as f: + contents = """ +type: hierarchical +backends: + - id: files1 + type: disk + weight: 1 + files_dir: "${temp_directory}/files1" + extra_dirs: + - type: temp + path: "${temp_directory}/tmp1" + - type: job_work + path: "${temp_directory}/job_working_directory1" + - id: files2 + type: disk + weight: 1 + files_dir: "${temp_directory}/files2" + extra_dirs: + - type: temp + path: "${temp_directory}/tmp2" + - type: job_work + path: "${temp_directory}/job_working_directory2" +""" + contents_template = string.Template(contents) + expanded_contents = contents_template.safe_substitute(temp_directory=tmpdir) + f.write(expanded_contents) + config["object_store_config_file"] = object_store_config + if datatypes_conf is not None: config['datatypes_config_file'] = datatypes_conf if enable_tool_shed_check: From 357adcacb6089d3f2a78b9abce7b4d5bcf5d1a71 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Thu, 18 Apr 2019 14:34:25 -0400 Subject: [PATCH 2/4] Don't calculate real path in tool action code if unused. --- lib/galaxy/tools/wrappers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tools/wrappers.py b/lib/galaxy/tools/wrappers.py index 474a54b60f5..8e23cb72585 100644 --- a/lib/galaxy/tools/wrappers.py +++ b/lib/galaxy/tools/wrappers.py @@ -322,7 +322,7 @@ class HasDatasets(object): def _dataset_wrapper(self, dataset, dataset_paths, **kwargs): wrapper_kwds = kwargs.copy() - if dataset: + if dataset and dataset_paths: real_path = dataset.file_name if real_path in dataset_paths: wrapper_kwds["dataset_path"] = dataset_paths[real_path] From b777e1b5d8a89f887230d2490d99ea87932eb68c Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 12 May 2019 20:13:57 +0200 Subject: [PATCH 3/4] Raise ObjectNotFound if file does not exist in object store And return an empty string if a path does not exist when calling `get_filename` or` get_extra_files_path` on a Dataset. --- lib/galaxy/model/__init__.py | 18 ++++++++++++------ lib/galaxy/objectstore/__init__.py | 5 ++++- test/unit/jobs/test_job_wrapper.py | 3 +++ test/unit/test_model_store.py | 1 + test/unit/tools/test_actions.py | 3 +++ .../tools/test_collect_primary_datasets.py | 3 +++ 6 files changed, 26 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index faebe5498d2..f0a3b589ee3 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -42,6 +42,7 @@ from sqlalchemy.orm import ( ) from sqlalchemy.schema import UniqueConstraint +import galaxy.exceptions import galaxy.model.metadata import galaxy.model.orm.now import galaxy.model.tags @@ -2156,8 +2157,10 @@ class Dataset(StorableObject, RepresentById): def get_file_name(self): if not self.external_filename: assert self.object_store is not None, "Object Store has not been initialized for dataset %s" % self.id - filename = self.object_store.get_filename(self) - return filename + if self.object_store.exists(self): + return self.object_store.get_filename(self) + else: + return '' else: filename = self.external_filename # Make filename absolute @@ -2175,7 +2178,9 @@ class Dataset(StorableObject, RepresentById): # actual database column so if SA instantiates this object - the # attribute won't exist yet. if not getattr(self, "external_extra_files_path", None): - return self.object_store.get_filename(self, dir_only=True, extra_dir=self._extra_files_rel_path) + if self.object_store.exists(self, dir_only=True, extra_dir=self._extra_files_rel_path): + return self.object_store.get_filename(self, dir_only=True, extra_dir=self._extra_files_rel_path) + return '' else: return os.path.abspath(self.external_extra_files_path) @@ -2266,11 +2271,12 @@ class Dataset(StorableObject, RepresentById): def full_delete(self): """Remove the file and extra files, marks deleted and purged""" # os.unlink( self.file_name ) - self.object_store.delete(self) + try: + self.object_store.delete(self) + except galaxy.exceptions.ObjectNotFound: + pass if self.object_store.exists(self, extra_dir=self._extra_files_rel_path, dir_only=True): self.object_store.delete(self, entire_dir=True, extra_dir=self._extra_files_rel_path, dir_only=True) - # if os.path.exists( self.extra_files_path ): - # shutil.rmtree( self.extra_files_path ) # TODO: purge metadata files self.deleted = True self.purged = True diff --git a/lib/galaxy/objectstore/__init__.py b/lib/galaxy/objectstore/__init__.py index b73a4a8ad56..6294ff77832 100644 --- a/lib/galaxy/objectstore/__init__.py +++ b/lib/galaxy/objectstore/__init__.py @@ -467,7 +467,10 @@ class DiskObjectStore(ObjectStore): # construct and return hashed path if os.path.exists(path): return path - return self._construct_path(obj, **kwargs) + path = self._construct_path(obj, **kwargs) + if not os.path.exists(path): + raise ObjectNotFound + return path def update_from_file(self, obj, file_name=None, create=False, **kwargs): """`create` parameter is not used in this implementation.""" diff --git a/test/unit/jobs/test_job_wrapper.py b/test/unit/jobs/test_job_wrapper.py index 5e370587116..50389f204ef 100644 --- a/test/unit/jobs/test_job_wrapper.py +++ b/test/unit/jobs/test_job_wrapper.py @@ -191,6 +191,9 @@ class MockObjectStore(object): def create(self, *args, **kwds): pass + def exists(self, *args, **kwargs): + return True + def get_filename(self, *args, **kwds): if kwds.get("base_dir", "") == "job_work": return self.working_directory diff --git a/test/unit/test_model_store.py b/test/unit/test_model_store.py index 8614658bfe7..ec2cb414c99 100644 --- a/test/unit/test_model_store.py +++ b/test/unit/test_model_store.py @@ -283,6 +283,7 @@ def test_import_export_composite_datasets(): h = model.History(name="Test History", user=u) d1 = _create_datasets(sa_session, h, 1, extension="html")[0] + app.object_store.create(d1.dataset, dir_only=True, extra_dir=d1.dataset._extra_files_rel_path) sa_session.add_all((h, d1)) sa_session.flush() diff --git a/test/unit/tools/test_actions.py b/test/unit/tools/test_actions.py index 3c23507ab6b..8628cd21205 100644 --- a/test/unit/tools/test_actions.py +++ b/test/unit/tools/test_actions.py @@ -274,6 +274,9 @@ class MockObjectStore(object): self.first_create = True self.object_store_id = "mycoolid" + def exists(self, *args, **kwargs): + return True + def create(self, dataset): self.created_datasets.append(dataset) if self.first_create: diff --git a/test/unit/tools/test_collect_primary_datasets.py b/test/unit/tools/test_collect_primary_datasets.py index 0f1814680b3..b926502e03f 100644 --- a/test/unit/tools/test_collect_primary_datasets.py +++ b/test/unit/tools/test_collect_primary_datasets.py @@ -408,6 +408,9 @@ class MockObjectStore(object): path = self.created_datasets[dataset] return os.stat(path).st_size + def exists(self, *args, **kwargs): + return True + def get_filename(self, dataset): return self.created_datasets[dataset] From 3d2c243b4dd2ccfa8943d4a59fba01d58c0cd414 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 May 2019 14:04:23 +0200 Subject: [PATCH 4/4] Explicitly create extra_files_path when discovering outputs with extra files --- lib/galaxy/model/__init__.py | 4 ++++ lib/galaxy/tools/parameters/output_collect.py | 1 + test/unit/test_model_store.py | 2 +- 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index f0a3b589ee3..1448ce6f75e 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -2184,6 +2184,10 @@ class Dataset(StorableObject, RepresentById): else: return os.path.abspath(self.external_extra_files_path) + def create_extra_files_path(self): + if not self.extra_files_path_exists(): + self.object_store.create(self, dir_only=True, extra_dir=self._extra_files_rel_path) + def set_extra_files_path(self, extra_files_path): if not extra_files_path: self.external_extra_files_path = None diff --git a/lib/galaxy/tools/parameters/output_collect.py b/lib/galaxy/tools/parameters/output_collect.py index caca15031cd..81c10a8d22d 100644 --- a/lib/galaxy/tools/parameters/output_collect.py +++ b/lib/galaxy/tools/parameters/output_collect.py @@ -316,6 +316,7 @@ def collect_primary_datasets(job_context, output, input_ext): extra_files_path = new_primary_datasets_attributes.get('extra_files', None) if extra_files_path: extra_files_path_joined = os.path.join(job_working_directory, extra_files_path) + primary_data.dataset.create_extra_files_path() for root, dirs, files in os.walk(extra_files_path_joined): extra_dir = os.path.join(primary_data.extra_files_path, root.replace(extra_files_path_joined, '', 1).lstrip(os.path.sep)) extra_dir = os.path.normpath(extra_dir) diff --git a/test/unit/test_model_store.py b/test/unit/test_model_store.py index ec2cb414c99..4f26f5b2574 100644 --- a/test/unit/test_model_store.py +++ b/test/unit/test_model_store.py @@ -283,7 +283,7 @@ def test_import_export_composite_datasets(): h = model.History(name="Test History", user=u) d1 = _create_datasets(sa_session, h, 1, extension="html")[0] - app.object_store.create(d1.dataset, dir_only=True, extra_dir=d1.dataset._extra_files_rel_path) + d1.dataset.create_extra_files_path() sa_session.add_all((h, d1)) sa_session.flush()