From 37dfd4292a69e4f76dcec785361649956eb1c8ca Mon Sep 17 00:00:00 2001 From: Helena Rasche Date: Fri, 1 Feb 2019 18:45:52 +0100 Subject: [PATCH 01/13] Fix? --- lib/galaxy/objectstore/s3.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py index a2577e8464e..09e78626e24 100644 --- a/lib/galaxy/objectstore/s3.py +++ b/lib/galaxy/objectstore/s3.py @@ -378,8 +378,6 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): except S3ResponseError: log.exception("Trouble checking existence of S3 key '%s'", rel_path) return False - if rel_path[0] == '/': - raise return exists def _in_cache(self, rel_path): From 20aaca36b1ed14487daedaea131a64e51f6bad66 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 12:37:05 +0100 Subject: [PATCH 02/13] Add an integration test case for swift/s3 --- .../objectstore/test_swift_objectstore.py | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 test/integration/objectstore/test_swift_objectstore.py diff --git a/test/integration/objectstore/test_swift_objectstore.py b/test/integration/objectstore/test_swift_objectstore.py new file mode 100644 index 00000000000..91b778d263b --- /dev/null +++ b/test/integration/objectstore/test_swift_objectstore.py @@ -0,0 +1,79 @@ + +import os +import string + +from galaxy_test.driver import integration_util + +OBJECT_STORE_HOST = os.environ.get('GALAXY_INTEGRATION_OBJECT_STORE_HOST', '127.0.0.1') +OBJECT_STORE_PORT = int(os.environ.get('GALAXY_INTEGRATION_OBJECT_STORE_PORT', 9000)) +OBJECT_STORE_ACCESS_KEY = os.environ.get('GALAXY_INTEGRATION_OBJECT_STORE_ACCESS_KEY', 'minioadmin') +OBJECT_STORE_SECRET_KEY = os.environ.get('GALAXY_INTEGRATION_OBJECT_STORE_SECRET_KEY', 'minioadmin') +OBJECT_STORE_CONFIG = string.Template(""" + + + + + + + + + + + + +""") +TEST_TOOL_IDS = [ + "multi_output", + "multi_output_configured", + "multi_output_assign_primary", + "multi_output_recurse", + "tool_provided_metadata_1", + "tool_provided_metadata_2", + "tool_provided_metadata_3", + "tool_provided_metadata_4", + "tool_provided_metadata_5", + "tool_provided_metadata_6", + "tool_provided_metadata_7", + "tool_provided_metadata_8", + "tool_provided_metadata_9", + "tool_provided_metadata_10", + "tool_provided_metadata_11", + "tool_provided_metadata_12", + "composite_output", + "composite_output_tests", + "metadata", + "metadata_bam", + "output_format", + "output_auto_format", +] + + +class SwiftObjectStoreIntegrationTestCase(integration_util.IntegrationTestCase): + + @classmethod + def handle_galaxy_config_kwds(cls, config): + temp_directory = cls._test_driver.mkdtemp() + cls.object_stores_parent = temp_directory + config_path = os.path.join(temp_directory, "object_store_conf.xml") + config["object_store_store_by"] = "uuid" + config["metadata_strategy"] = "extended" + config["outpus_to_working_dir"] = True + config["retry_metadata_internally"] = False + with open(config_path, "w") as f: + f.write( + OBJECT_STORE_CONFIG.safe_substitute( + { + "temp_directory": temp_directory, + "host": OBJECT_STORE_HOST, + "port": OBJECT_STORE_PORT, + "access_key": OBJECT_STORE_ACCESS_KEY, + "secret_key": OBJECT_STORE_SECRET_KEY, + } + ) + ) + config["object_store_config_file"] = config_path + + +instance = integration_util.integration_module_instance(SwiftObjectStoreIntegrationTestCase) + +test_tools = integration_util.integration_tool_runner(TEST_TOOL_IDS) \ No newline at end of file From 8fef712a5d67e8f6fafef91816cae5b6e0e1722c Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 12:57:40 +0100 Subject: [PATCH 03/13] Don't use mutable default argument --- lib/galaxy/objectstore/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/objectstore/__init__.py b/lib/galaxy/objectstore/__init__.py index ce8f2a579b5..d0dc0d6ac48 100644 --- a/lib/galaxy/objectstore/__init__.py +++ b/lib/galaxy/objectstore/__init__.py @@ -82,7 +82,7 @@ class ObjectStore(object): 000/obj.id) """ - def __init__(self, config, config_dict={}, **kwargs): + def __init__(self, config, config_dict=None, **kwargs): """ :type config: object :param config: An object, most likely populated from @@ -95,6 +95,8 @@ class ObjectStore(object): parent directory those directories will be created. * new_file_path -- Used to set the 'temp' extra_dir. """ + if config_dict is None: + config_dict = {} self.running = True self.config = config self.check_old_style = config.object_store_check_old_style From 6a56f3a5deb51dd1fafa8c588f32b14e67d10245 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 12:58:48 +0100 Subject: [PATCH 04/13] Disable cleanup thread in set_metadata.py --- lib/galaxy/objectstore/s3.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py index 09e78626e24..2da1d12b575 100644 --- a/lib/galaxy/objectstore/s3.py +++ b/lib/galaxy/objectstore/s3.py @@ -123,7 +123,7 @@ class CloudConfigMixin(object): 'conn_path': self.conn_path, }, 'cache': { - 'size': self.cache_size, + 'size': -1, # disable cache cleaning when starting an exported object store in set_metadata.py 'path': self.staging_path, } } From 57e84e78a38fda472067895e30b75d92c13084b6 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 13:00:37 +0100 Subject: [PATCH 05/13] Fix store_by_uuid and extended metadata collection for s3 obejct stores --- lib/galaxy/objectstore/s3.py | 36 +++++++++++++++++++----------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py index 2da1d12b575..992c7fb968b 100644 --- a/lib/galaxy/objectstore/s3.py +++ b/lib/galaxy/objectstore/s3.py @@ -138,7 +138,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): store_type = 's3' def __init__(self, config, config_dict): - super(S3ObjectStore, self).__init__(config) + super(S3ObjectStore, self).__init__(config, config_dict) self.transfer_progress = 0 @@ -162,6 +162,8 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): self.cache_size = cache_dict.get('size', -1) self.staging_path = cache_dict.get('path') or self.config.object_store_cache_path + self.store_by = config_dict.get("store_by", None) or getattr(config, "object_store_store_by", "id") + assert self.store_by in ["id", "uuid"] extra_dirs = dict( (e['type'], e['path']) for e in config_dict.get('extra_dirs', [])) @@ -187,7 +189,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): 'conn_path': self.conn_path} self._configure_connection() - self.bucket = self._get_bucket(self.bucket) + self.__bucket = self._get_bucket(self.bucket) # Clean cache only if value is set in galaxy.ini if self.cache_size != -1: # Convert GBs to bytes for comparison @@ -325,7 +327,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): # alt_name can contain parent directory references, but S3 will not # follow them, so if they are valid we normalize them out alt_name = os.path.normpath(alt_name) - rel_path = os.path.join(*directory_hash_id(obj.id)) + rel_path = os.path.join(*directory_hash_id(self._get_object_id(obj))) if extra_dir is not None: if extra_dir_at_root: rel_path = os.path.join(extra_dir, rel_path) @@ -334,7 +336,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): # for JOB_WORK directory if obj_dir: - rel_path = os.path.join(rel_path, str(obj.id)) + rel_path = os.path.join(rel_path, str(self._get_object_id(obj))) if base_dir: base = self.extra_dirs.get(base_dir) return os.path.join(base, rel_path) @@ -343,7 +345,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): rel_path = '%s/' % rel_path if not dir_only: - rel_path = os.path.join(rel_path, alt_name if alt_name else "dataset_%s.dat" % obj.id) + rel_path = os.path.join(rel_path, alt_name if alt_name else "dataset_%s.dat" % self._get_object_id(obj)) return rel_path def _get_cache_path(self, rel_path): @@ -354,7 +356,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): def _get_size_in_s3(self, rel_path): try: - key = self.bucket.get_key(rel_path) + key = self.__bucket.get_key(rel_path) if key: return key.size except S3ResponseError: @@ -367,13 +369,13 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): # A hackish way of testing if the rel_path is a folder vs a file is_dir = rel_path[-1] == '/' if is_dir: - keyresult = self.bucket.get_all_keys(prefix=rel_path) + keyresult = self.__bucket.get_all_keys(prefix=rel_path) if len(keyresult) > 0: exists = True else: exists = False else: - key = Key(self.bucket, rel_path) + key = Key(self.__bucket, rel_path) exists = key.exists() except S3ResponseError: log.exception("Trouble checking existence of S3 key '%s'", rel_path) @@ -425,7 +427,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): def _download(self, rel_path): try: log.debug("Pulling key '%s' into cache to %s", rel_path, self._get_cache_path(rel_path)) - key = self.bucket.get_key(rel_path) + key = self.__bucket.get_key(rel_path) # Test if cache is large enough to hold the new file if self.cache_size > 0 and key.size > self.cache_size: log.critical("File %s is larger (%s) than the cache size (%s). Cannot download.", @@ -444,7 +446,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): key.get_contents_to_filename(self._get_cache_path(rel_path), cb=self._transfer_cb, num_cb=10) return True except S3ResponseError: - log.exception("Problem downloading key '%s' from S3 bucket '%s'", rel_path, self.bucket.name) + log.exception("Problem downloading key '%s' from S3 bucket '%s'", rel_path, self.__bucket.name) return False def _push_to_os(self, rel_path, source_file=None, from_string=None): @@ -458,7 +460,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): try: source_file = source_file if source_file else self._get_cache_path(rel_path) if os.path.exists(source_file): - key = Key(self.bucket, rel_path) + key = Key(self.__bucket, rel_path) if os.path.getsize(source_file) == 0 and key.exists(): log.debug("Wanted to push file '%s' to S3 key '%s' but its size is 0; skipping.", source_file, rel_path) return True @@ -476,7 +478,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): cb=self._transfer_cb, num_cb=10) else: - multipart_upload(self.s3server, self.bucket, key.name, source_file, mb_size) + multipart_upload(self.s3server, self.__bucket, key.name, source_file, mb_size) end_time = datetime.now() log.debug("Pushed cache file '%s' to key '%s' (%s bytes transfered in %s sec)", source_file, rel_path, os.path.getsize(source_file), end_time - start_time) @@ -545,7 +547,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): alt_name = kwargs.get('alt_name', None) # Construct hashed path - rel_path = os.path.join(*directory_hash_id(obj.id)) + rel_path = os.path.join(*directory_hash_id(self._get_object_id(obj))) # Optionally append extra_dir if extra_dir is not None: @@ -566,7 +568,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): # self._push_to_os(s3_dir, from_string='') # If instructed, create the dataset in cache & in S3 if not dir_only: - rel_path = os.path.join(rel_path, alt_name if alt_name else "dataset_%s.dat" % obj.id) + rel_path = os.path.join(rel_path, alt_name if alt_name else "dataset_%s.dat" % self._get_object_id(obj)) open(os.path.join(self.staging_path, rel_path), 'w').close() self._push_to_os(rel_path, from_string='') @@ -607,7 +609,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): # but requires iterating through each individual key in S3 and deleing it. if entire_dir and extra_dir: shutil.rmtree(self._get_cache_path(rel_path)) - results = self.bucket.get_all_keys(prefix=rel_path) + results = self.__bucket.get_all_keys(prefix=rel_path) for key in results: log.debug("Deleting key %s", key.name) key.delete() @@ -617,7 +619,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): os.unlink(self._get_cache_path(rel_path)) # Delete from S3 as well if self._key_exists(rel_path): - key = Key(self.bucket, rel_path) + key = Key(self.__bucket, rel_path) log.debug("Deleting key %s", key.name) key.delete() return True @@ -705,7 +707,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): if self.exists(obj, **kwargs): rel_path = self._construct_path(obj, **kwargs) try: - key = Key(self.bucket, rel_path) + key = Key(self.__bucket, rel_path) return key.generate_url(expires_in=86400) # 24hrs except S3ResponseError: log.exception("Trouble generating URL for dataset '%s'", rel_path) From 0a252ed4f184dd1a5c46b655c4d418173c999cce Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 13:01:13 +0100 Subject: [PATCH 06/13] Manage minio docker container in integration test --- .../objectstore/test_swift_objectstore.py | 36 ++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/test/integration/objectstore/test_swift_objectstore.py b/test/integration/objectstore/test_swift_objectstore.py index 91b778d263b..a2efd076d54 100644 --- a/test/integration/objectstore/test_swift_objectstore.py +++ b/test/integration/objectstore/test_swift_objectstore.py @@ -1,6 +1,7 @@ import os import string +import subprocess from galaxy_test.driver import integration_util @@ -48,11 +49,44 @@ TEST_TOOL_IDS = [ ] +def start_minio(container_name): + minio_start_args = [ + 'docker', + 'run', + '-p', + '{port}:9000'.format(port=OBJECT_STORE_PORT), + '-d', + '--name', + container_name, + # '--rm', + 'minio/minio:latest', + 'server', + '/tmp/data'] + subprocess.check_call(minio_start_args) + + +def stop_minio(container_name): + subprocess.check_call(['docker', 'stop', container_name]) + + +@integration_util.skip_unless_docker() class SwiftObjectStoreIntegrationTestCase(integration_util.IntegrationTestCase): + @classmethod + def setUpClass(cls): + cls.container_name = "%s_container" % cls.__name__ + start_minio(cls.container_name) + super(SwiftObjectStoreIntegrationTestCase, cls).setUpClass() + + @classmethod + def tearDownClass(cls): + stop_minio(cls.container_name) + super(SwiftObjectStoreIntegrationTestCase, cls).tearDownClass() + @classmethod def handle_galaxy_config_kwds(cls, config): temp_directory = cls._test_driver.mkdtemp() + cls.container_name = os.path.basename(temp_directory) cls.object_stores_parent = temp_directory config_path = os.path.join(temp_directory, "object_store_conf.xml") config["object_store_store_by"] = "uuid" @@ -76,4 +110,4 @@ class SwiftObjectStoreIntegrationTestCase(integration_util.IntegrationTestCase): instance = integration_util.integration_module_instance(SwiftObjectStoreIntegrationTestCase) -test_tools = integration_util.integration_tool_runner(TEST_TOOL_IDS) \ No newline at end of file +test_tools = integration_util.integration_tool_runner(TEST_TOOL_IDS) From 7a63fd1db202a9993c09cb6f10c0615dc8dc5c99 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 14:16:01 +0100 Subject: [PATCH 07/13] Fix S3 object store shutdown --- lib/galaxy/objectstore/s3.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py index 992c7fb968b..71d11b7a1cf 100644 --- a/lib/galaxy/objectstore/s3.py +++ b/lib/galaxy/objectstore/s3.py @@ -716,6 +716,14 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): def get_store_usage_percent(self): return 0.0 + def shutdown(self): + self.running = False + thread = getattr(self, 'cache_monitor_thread', None) + if thread: + log.debug("Shutting down thread") + self.sleeper.wake() + thread.join(5) + class SwiftObjectStore(S3ObjectStore): """ From b5dd8346fbe8b8bdd88eaf77566a707b9d5b3c94 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 14:18:49 +0100 Subject: [PATCH 08/13] Fix test container shutdown --- test/integration/objectstore/test_swift_objectstore.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/integration/objectstore/test_swift_objectstore.py b/test/integration/objectstore/test_swift_objectstore.py index a2efd076d54..0ee10a4b462 100644 --- a/test/integration/objectstore/test_swift_objectstore.py +++ b/test/integration/objectstore/test_swift_objectstore.py @@ -66,7 +66,7 @@ def start_minio(container_name): def stop_minio(container_name): - subprocess.check_call(['docker', 'stop', container_name]) + subprocess.check_call(['docker', 'rm', '-f', container_name]) @integration_util.skip_unless_docker() @@ -86,7 +86,6 @@ class SwiftObjectStoreIntegrationTestCase(integration_util.IntegrationTestCase): @classmethod def handle_galaxy_config_kwds(cls, config): temp_directory = cls._test_driver.mkdtemp() - cls.container_name = os.path.basename(temp_directory) cls.object_stores_parent = temp_directory config_path = os.path.join(temp_directory, "object_store_conf.xml") config["object_store_store_by"] = "uuid" From 510504075f2c079fe746414b27e2b206dde22652 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 14:22:54 +0100 Subject: [PATCH 09/13] Don't run extended metadata strategy --- test/integration/objectstore/test_swift_objectstore.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/objectstore/test_swift_objectstore.py b/test/integration/objectstore/test_swift_objectstore.py index 0ee10a4b462..c3e90af1506 100644 --- a/test/integration/objectstore/test_swift_objectstore.py +++ b/test/integration/objectstore/test_swift_objectstore.py @@ -89,7 +89,8 @@ class SwiftObjectStoreIntegrationTestCase(integration_util.IntegrationTestCase): cls.object_stores_parent = temp_directory config_path = os.path.join(temp_directory, "object_store_conf.xml") config["object_store_store_by"] = "uuid" - config["metadata_strategy"] = "extended" + # This doesn't quite work yet, fails with extra_files_path + # config["metadata_strategy"] = "extended" config["outpus_to_working_dir"] = True config["retry_metadata_internally"] = False with open(config_path, "w") as f: From 46f3af60450098a5a7846edb2f1fec31873cc7d4 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 15:20:17 +0100 Subject: [PATCH 10/13] Use single underscore bucket --- lib/galaxy/objectstore/s3.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py index 71d11b7a1cf..30d5f1100e1 100644 --- a/lib/galaxy/objectstore/s3.py +++ b/lib/galaxy/objectstore/s3.py @@ -189,7 +189,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): 'conn_path': self.conn_path} self._configure_connection() - self.__bucket = self._get_bucket(self.bucket) + self._bucket = self._get_bucket(self.bucket) # Clean cache only if value is set in galaxy.ini if self.cache_size != -1: # Convert GBs to bytes for comparison @@ -356,7 +356,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): def _get_size_in_s3(self, rel_path): try: - key = self.__bucket.get_key(rel_path) + key = self._bucket.get_key(rel_path) if key: return key.size except S3ResponseError: @@ -369,13 +369,13 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): # A hackish way of testing if the rel_path is a folder vs a file is_dir = rel_path[-1] == '/' if is_dir: - keyresult = self.__bucket.get_all_keys(prefix=rel_path) + keyresult = self._bucket.get_all_keys(prefix=rel_path) if len(keyresult) > 0: exists = True else: exists = False else: - key = Key(self.__bucket, rel_path) + key = Key(self._bucket, rel_path) exists = key.exists() except S3ResponseError: log.exception("Trouble checking existence of S3 key '%s'", rel_path) @@ -427,7 +427,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): def _download(self, rel_path): try: log.debug("Pulling key '%s' into cache to %s", rel_path, self._get_cache_path(rel_path)) - key = self.__bucket.get_key(rel_path) + key = self._bucket.get_key(rel_path) # Test if cache is large enough to hold the new file if self.cache_size > 0 and key.size > self.cache_size: log.critical("File %s is larger (%s) than the cache size (%s). Cannot download.", @@ -446,7 +446,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): key.get_contents_to_filename(self._get_cache_path(rel_path), cb=self._transfer_cb, num_cb=10) return True except S3ResponseError: - log.exception("Problem downloading key '%s' from S3 bucket '%s'", rel_path, self.__bucket.name) + log.exception("Problem downloading key '%s' from S3 bucket '%s'", rel_path, self._bucket.name) return False def _push_to_os(self, rel_path, source_file=None, from_string=None): @@ -460,7 +460,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): try: source_file = source_file if source_file else self._get_cache_path(rel_path) if os.path.exists(source_file): - key = Key(self.__bucket, rel_path) + key = Key(self._bucket, rel_path) if os.path.getsize(source_file) == 0 and key.exists(): log.debug("Wanted to push file '%s' to S3 key '%s' but its size is 0; skipping.", source_file, rel_path) return True @@ -478,7 +478,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): cb=self._transfer_cb, num_cb=10) else: - multipart_upload(self.s3server, self.__bucket, key.name, source_file, mb_size) + multipart_upload(self.s3server, self._bucket, key.name, source_file, mb_size) end_time = datetime.now() log.debug("Pushed cache file '%s' to key '%s' (%s bytes transfered in %s sec)", source_file, rel_path, os.path.getsize(source_file), end_time - start_time) @@ -609,7 +609,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): # but requires iterating through each individual key in S3 and deleing it. if entire_dir and extra_dir: shutil.rmtree(self._get_cache_path(rel_path)) - results = self.__bucket.get_all_keys(prefix=rel_path) + results = self._bucket.get_all_keys(prefix=rel_path) for key in results: log.debug("Deleting key %s", key.name) key.delete() @@ -619,7 +619,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): os.unlink(self._get_cache_path(rel_path)) # Delete from S3 as well if self._key_exists(rel_path): - key = Key(self.__bucket, rel_path) + key = Key(self._bucket, rel_path) log.debug("Deleting key %s", key.name) key.delete() return True @@ -707,7 +707,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): if self.exists(obj, **kwargs): rel_path = self._construct_path(obj, **kwargs) try: - key = Key(self.__bucket, rel_path) + key = Key(self._bucket, rel_path) return key.generate_url(expires_in=86400) # 24hrs except S3ResponseError: log.exception("Trouble generating URL for dataset '%s'", rel_path) From 494c15cb9d622c3c75bbcf72b880ee386cbe01f5 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 18:31:06 +0100 Subject: [PATCH 11/13] Be more explcit about starting cache monitor thread --- lib/galaxy/objectstore/s3.py | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py index 30d5f1100e1..c6049441a03 100644 --- a/lib/galaxy/objectstore/s3.py +++ b/lib/galaxy/objectstore/s3.py @@ -123,9 +123,10 @@ class CloudConfigMixin(object): 'conn_path': self.conn_path, }, 'cache': { - 'size': -1, # disable cache cleaning when starting an exported object store in set_metadata.py + 'size': self.cache_size, 'path': self.staging_path, - } + }, + 'enable_cache_monitor': False, } @@ -146,6 +147,7 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): bucket_dict = config_dict['bucket'] connection_dict = config_dict.get('connection', {}) cache_dict = config_dict['cache'] + self.enable_cache_monitor = config_dict.get('enable_cache_monitor', True) self.access_key = auth_dict.get('access_key') self.secret_key = auth_dict.get('secret_key') @@ -190,8 +192,16 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): self._configure_connection() self._bucket = self._get_bucket(self.bucket) + self.start_cache_monitor() + # Test if 'axel' is available for parallel download and pull the key into cache + if which('axel'): + self.use_axel = True + else: + self.use_axel = False + + def start_cache_monitor(self): # Clean cache only if value is set in galaxy.ini - if self.cache_size != -1: + if self.cache_size != -1 and self.enable_cache_monitor: # Convert GBs to bytes for comparison self.cache_size = self.cache_size * 1073741824 # Helper for interruptable sleep @@ -199,11 +209,6 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): self.cache_monitor_thread = threading.Thread(target=self.__cache_monitor) self.cache_monitor_thread.start() log.info("Cache cleaner manager started") - # Test if 'axel' is available for parallel download and pull the key into cache - if which('axel'): - self.use_axel = True - else: - self.use_axel = False def _configure_connection(self): log.debug("Configuring S3 Connection") From 6d2e2ef10bde3812dfe292ae1074b08fd501033d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 18:31:58 +0100 Subject: [PATCH 12/13] Drop redundant store_by attribute --- lib/galaxy/objectstore/s3.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py index c6049441a03..9c53c0aed29 100644 --- a/lib/galaxy/objectstore/s3.py +++ b/lib/galaxy/objectstore/s3.py @@ -164,8 +164,6 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): self.cache_size = cache_dict.get('size', -1) self.staging_path = cache_dict.get('path') or self.config.object_store_cache_path - self.store_by = config_dict.get("store_by", None) or getattr(config, "object_store_store_by", "id") - assert self.store_by in ["id", "uuid"] extra_dirs = dict( (e['type'], e['path']) for e in config_dict.get('extra_dirs', [])) From 3623f226f1474cb233e74bf6ae7e42a71012d300 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 18:32:10 +0100 Subject: [PATCH 13/13] Drop badly formatted logging --- lib/galaxy/objectstore/s3.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py index 9c53c0aed29..327988be7be 100644 --- a/lib/galaxy/objectstore/s3.py +++ b/lib/galaxy/objectstore/s3.py @@ -169,9 +169,6 @@ class S3ObjectStore(ObjectStore, CloudConfigMixin): (e['type'], e['path']) for e in config_dict.get('extra_dirs', [])) self.extra_dirs.update(extra_dirs) - log.debug("Object cache dir: %s", self.staging_path) - log.debug(" job work dir: %s", self.extra_dirs['job_work']) - self._initialize() def _initialize(self):