diff --git a/lib/galaxy/config/schemas/config_schema.yml b/lib/galaxy/config/schemas/config_schema.yml index 2c08a572191..6d32efcfa7b 100644 --- a/lib/galaxy/config/schemas/config_schema.yml +++ b/lib/galaxy/config/schemas/config_schema.yml @@ -974,6 +974,23 @@ mapping: Configuration file for the object store If this is set and exists, it overrides any other objectstore settings. + object_store_cache_path: + type: str + default: object_store_cache + path_resolves_to: cache_dir + required: false + desc: | + Default cache path for caching object stores if cache not configured for + that object store entry. + + object_store_cache_size: + type: int + default: -1 + required: false + desc: | + Default cache size for caching object stores if cache not configured for + that object store entry. + object_store_store_by: type: str required: false diff --git a/lib/galaxy/objectstore/azure_blob.py b/lib/galaxy/objectstore/azure_blob.py index 85e38ebcd9f..00e9b9f2e97 100644 --- a/lib/galaxy/objectstore/azure_blob.py +++ b/lib/galaxy/objectstore/azure_blob.py @@ -30,6 +30,7 @@ from . import ConcreteObjectStore from .caching import ( CacheTarget, InProcessCacheMonitor, + parse_caching_config_dict_from_xml, ) NO_BLOBSERVICE_ERROR_MESSAGE = ( @@ -51,9 +52,7 @@ def parse_config_xml(config_xml): container_name = container_xml.get("name") max_chunk_size = int(container_xml.get("max_chunk_size", 250)) # currently unused - c_xml = config_xml.findall("cache")[0] - cache_size = float(c_xml.get("size", -1)) - staging_path = c_xml.get("path", None) + cache_dict = parse_caching_config_dict_from_xml(config_xml) tag, attrs = "extra_dir", ("type", "path") extra_dirs = config_xml.findall(tag) @@ -71,10 +70,7 @@ def parse_config_xml(config_xml): "name": container_name, "max_chunk_size": max_chunk_size, }, - "cache": { - "size": cache_size, - "path": staging_path, - }, + "cache": cache_dict, "extra_dirs": extra_dirs, "private": ConcreteObjectStore.parse_private_from_config_xml(config_xml), } @@ -101,7 +97,7 @@ class AzureBlobObjectStore(ConcreteObjectStore): auth_dict = config_dict["auth"] container_dict = config_dict["container"] - cache_dict = config_dict["cache"] + cache_dict = config_dict.get("cache") or {} self.enable_cache_monitor = config_dict.get("enable_cache_monitor", True) self.account_name = auth_dict.get("account_name") @@ -110,7 +106,7 @@ class AzureBlobObjectStore(ConcreteObjectStore): self.container_name = container_dict.get("name") self.max_chunk_size = container_dict.get("max_chunk_size", 250) # currently unused - self.cache_size = cache_dict.get("size", -1) + self.cache_size = cache_dict.get("size") or self.config.object_store_cache_size self.staging_path = cache_dict.get("path") or self.config.object_store_cache_path self._initialize() diff --git a/lib/galaxy/objectstore/caching.py b/lib/galaxy/objectstore/caching.py index d7fa078341d..36b9dd44748 100644 --- a/lib/galaxy/objectstore/caching.py +++ b/lib/galaxy/objectstore/caching.py @@ -98,6 +98,23 @@ def _get_cache_size_files(cache_path) -> Tuple[int, FileListT]: return cache_size, file_list +def parse_caching_config_dict_from_xml(config_xml): + cache_els = config_xml.findall("cache") + if len(cache_els) > 0: + c_xml = config_xml.findall("cache")[0] + cache_size = float(c_xml.get("size", -1)) + + staging_path = c_xml.get("path", None) + + cache_dict = { + "size": cache_size, + "path": staging_path, + } + else: + cache_dict = {} + return cache_dict + + class InProcessCacheMonitor: def __init__(self, cache_target: CacheTarget, interval: int = 30, initial_sleep: Optional[int] = 2): # This Event object is initialized to False diff --git a/lib/galaxy/objectstore/cloud.py b/lib/galaxy/objectstore/cloud.py index a3890f8cf5f..78be31c83d9 100644 --- a/lib/galaxy/objectstore/cloud.py +++ b/lib/galaxy/objectstore/cloud.py @@ -86,7 +86,7 @@ class Cloud(ConcreteObjectStore, CloudConfigMixin): bucket_dict = config_dict["bucket"] connection_dict = config_dict.get("connection", {}) - cache_dict = config_dict["cache"] + cache_dict = config_dict.get("cache") or {} self.enable_cache_monitor = config_dict.get("enable_cache_monitor", True) self.provider = config_dict["provider"] @@ -101,7 +101,7 @@ class Cloud(ConcreteObjectStore, CloudConfigMixin): self.is_secure = connection_dict.get("is_secure", True) self.conn_path = connection_dict.get("conn_path", "/") - self.cache_size = cache_dict.get("size", -1) + self.cache_size = cache_dict.get("size") or self.config.object_store_cache_size self.staging_path = cache_dict.get("path") or self.config.object_store_cache_path self._initialize() diff --git a/lib/galaxy/objectstore/irods.py b/lib/galaxy/objectstore/irods.py index 0313f37a315..b6a0450389e 100644 --- a/lib/galaxy/objectstore/irods.py +++ b/lib/galaxy/objectstore/irods.py @@ -202,10 +202,8 @@ class IRODSObjectStore(DiskObjectStore, CloudConfigMixin): if self.connection_pool_monitor_interval is None: _config_dict_error("connection->connection_pool_monitor_interval") - cache_dict = config_dict["cache"] - if cache_dict is None: - _config_dict_error("cache") - self.cache_size = cache_dict.get("size", -1) + cache_dict = config_dict.get("cache") or {} + self.cache_size = cache_dict.get("size") or self.config.object_store_cache_path if self.cache_size is None: _config_dict_error("cache->size") self.staging_path = cache_dict.get("path") or self.config.object_store_cache_path diff --git a/lib/galaxy/objectstore/s3.py b/lib/galaxy/objectstore/s3.py index 60e457fe200..2db7fd7e076 100644 --- a/lib/galaxy/objectstore/s3.py +++ b/lib/galaxy/objectstore/s3.py @@ -35,6 +35,7 @@ from . import ConcreteObjectStore from .caching import ( CacheTarget, InProcessCacheMonitor, + parse_caching_config_dict_from_xml, ) from .s3_multipart_upload import multipart_upload @@ -70,10 +71,7 @@ def parse_config_xml(config_xml): is_secure = string_as_bool(cn_xml.get("is_secure", "True")) conn_path = cn_xml.get("conn_path", "/") - c_xml = config_xml.findall("cache")[0] - cache_size = float(c_xml.get("size", -1)) - - staging_path = c_xml.get("path", None) + cache_dict = parse_caching_config_dict_from_xml(config_xml) tag, attrs = "extra_dir", ("type", "path") extra_dirs = config_xml.findall(tag) @@ -100,10 +98,7 @@ def parse_config_xml(config_xml): "is_secure": is_secure, "conn_path": conn_path, }, - "cache": { - "size": cache_size, - "path": staging_path, - }, + "cache": cache_dict, "extra_dirs": extra_dirs, "private": ConcreteObjectStore.parse_private_from_config_xml(config_xml), } @@ -158,7 +153,7 @@ class S3ObjectStore(ConcreteObjectStore, CloudConfigMixin): auth_dict = config_dict["auth"] bucket_dict = config_dict["bucket"] connection_dict = config_dict.get("connection", {}) - cache_dict = config_dict["cache"] + cache_dict = config_dict.get("cache") or {} self.enable_cache_monitor = config_dict.get("enable_cache_monitor", True) self.access_key = auth_dict.get("access_key") @@ -174,7 +169,7 @@ class S3ObjectStore(ConcreteObjectStore, CloudConfigMixin): self.is_secure = connection_dict.get("is_secure", True) self.conn_path = connection_dict.get("conn_path", "/") - self.cache_size = cache_dict.get("size", -1) + self.cache_size = cache_dict.get("size") or self.config.object_store_cache_size self.staging_path = cache_dict.get("path") or self.config.object_store_cache_path extra_dirs = {e["type"]: e["path"] for e in config_dict.get("extra_dirs", [])} diff --git a/lib/galaxy/objectstore/unittest_utils/__init__.py b/lib/galaxy/objectstore/unittest_utils/__init__.py index 2cc0239aef2..e6acadc4516 100644 --- a/lib/galaxy/objectstore/unittest_utils/__init__.py +++ b/lib/galaxy/objectstore/unittest_utils/__init__.py @@ -39,6 +39,7 @@ class Config: config_file = "store.yaml" self.write(config_str, config_file) config = MockConfig(self.temp_directory, config_file, store_by=store_by) + self.global_config = config if clazz is None: self.object_store = objectstore.build_object_store_from_config(config) elif config_file == "store.xml": @@ -68,6 +69,7 @@ class MockConfig: self.file_path = temp_directory self.object_store_config_file = os.path.join(temp_directory, config_file) self.object_store_check_old_style = False + self.object_store_cache_size = -1 self.object_store_cache_path = os.path.join(temp_directory, "staging") self.object_store_store_by = store_by self.jobs_directory = temp_directory diff --git a/test/unit/objectstore/test_objectstore.py b/test/unit/objectstore/test_objectstore.py index 449a94425be..8deba551543 100644 --- a/test/unit/objectstore/test_objectstore.py +++ b/test/unit/objectstore/test_objectstore.py @@ -600,22 +600,22 @@ def test_hiercachical_backend_must_share_quota_source(): # Unit testing the cloud and advanced infrastructure object stores is difficult, but # we can at least stub out initializing and test the configuration of these things from # XML and dicts. -class UnitializedPithosObjectStore(PithosObjectStore): +class UninitializedPithosObjectStore(PithosObjectStore): def _initialize(self): pass -class UnitializeS3ObjectStore(S3ObjectStore): +class UninitializedS3ObjectStore(S3ObjectStore): def _initialize(self): pass -class UnitializedAzureBlobObjectStore(AzureBlobObjectStore): +class UninitializedAzureBlobObjectStore(AzureBlobObjectStore): def _initialize(self): pass -class UnitializedCloudObjectStore(Cloud): +class UninitializedCloudObjectStore(Cloud): def _initialize(self): pass @@ -650,7 +650,7 @@ extra_dirs: def test_config_parse_pithos(): for config_str in [PITHOS_TEST_CONFIG, PITHOS_TEST_CONFIG_YAML]: - with TestConfig(config_str, clazz=UnitializedPithosObjectStore) as (directory, object_store): + with TestConfig(config_str, clazz=UninitializedPithosObjectStore) as (directory, object_store): configured_config_dict = object_store.config_dict _assert_has_keys(configured_config_dict, ["auth", "container", "extra_dirs"]) @@ -719,7 +719,7 @@ extra_dirs: def test_config_parse_s3(): for config_str in [S3_TEST_CONFIG, S3_TEST_CONFIG_YAML]: - with TestConfig(config_str, clazz=UnitializeS3ObjectStore) as (directory, object_store): + with TestConfig(config_str, clazz=UninitializedS3ObjectStore) as (directory, object_store): assert object_store.private assert object_store.access_key == "access_moo" assert object_store.secret_key == "secret_cow" @@ -767,6 +767,41 @@ def test_config_parse_s3(): assert len(extra_dirs) == 2 +S3_DEFAULT_CACHE_TEST_CONFIG = """ + + + + + +""" + + +S3_DEFAULT_CACHE_TEST_CONFIG_YAML = """ +type: s3 +private: true +auth: + access_key: access_moo + secret_key: secret_cow + +bucket: + name: unique_bucket_name_all_lowercase + use_reduced_redundancy: false + +extra_dirs: +- type: job_work + path: database/job_working_directory_s3 +- type: temp + path: database/tmp_s3 +""" + + +def test_config_parse_s3_with_default_cache(): + for config_str in [S3_DEFAULT_CACHE_TEST_CONFIG, S3_DEFAULT_CACHE_TEST_CONFIG_YAML]: + with TestConfig(config_str, clazz=UninitializedS3ObjectStore) as (directory, object_store): + assert object_store.cache_size == -1 + assert object_store.staging_path == directory.global_config.object_store_cache_path + + CLOUD_AWS_TEST_CONFIG = """ @@ -882,7 +917,7 @@ def test_config_parse_cloud(): path = os.path.join(tmpdir, "gcp.config") open(path, "w").write("some_gcp_config") config_str = config_str.replace("gcp.config", path) - with TestConfig(config_str, clazz=UnitializedCloudObjectStore) as (directory, object_store): + with TestConfig(config_str, clazz=UninitializedCloudObjectStore) as (directory, object_store): assert object_store.bucket_name == "unique_bucket_name_all_lowercase" assert object_store.use_rr is False @@ -949,7 +984,7 @@ CLOUD_AWS_NO_AUTH_TEST_CONFIG = """ def test_config_parse_cloud_noauth_for_aws(): for config_str in [CLOUD_AWS_NO_AUTH_TEST_CONFIG]: - with TestConfig(config_str, clazz=UnitializedCloudObjectStore) as (directory, object_store): + with TestConfig(config_str, clazz=UninitializedCloudObjectStore) as (directory, object_store): assert object_store.bucket_name == "unique_bucket_name_all_lowercase" assert object_store.use_rr is False @@ -996,6 +1031,22 @@ def test_config_parse_cloud_noauth_for_aws(): assert len(extra_dirs) == 2 +CLOUD_AWS_NO_CACHE_TEST_CONFIG = """ + + + + + +""" + + +def test_config_parse_cloud_no_cache_for_aws(): + for config_str in [CLOUD_AWS_NO_CACHE_TEST_CONFIG]: + with TestConfig(config_str, clazz=UninitializedCloudObjectStore) as (directory, object_store): + assert object_store.staging_path == directory.global_config.object_store_cache_path + assert object_store.cache_size == -1 + + AZURE_BLOB_TEST_CONFIG = """ @@ -1030,7 +1081,7 @@ extra_dirs: def test_config_parse_azure(): for config_str in [AZURE_BLOB_TEST_CONFIG, AZURE_BLOB_TEST_CONFIG_YAML]: - with TestConfig(config_str, clazz=UnitializedAzureBlobObjectStore) as (directory, object_store): + with TestConfig(config_str, clazz=UninitializedAzureBlobObjectStore) as (directory, object_store): assert object_store.account_name == "azureact" assert object_store.account_key == "password123" @@ -1103,6 +1154,40 @@ def test_check_cache_sanity(tmp_path): assert not path.exists() +AZURE_BLOB_NO_CACHE_TEST_CONFIG = """ + + + + + +""" + + +AZURE_BLOB_NO_CACHE_TEST_CONFIG_YAML = """ +type: azure_blob +auth: + account_name: azureact + account_key: password123 + +container: + name: unique_container_name + max_chunk_size: 250 + +extra_dirs: +- type: job_work + path: database/job_working_directory_azure +- type: temp + path: database/tmp_azure +""" + + +def test_config_parse_azure_no_cache(): + for config_str in [AZURE_BLOB_NO_CACHE_TEST_CONFIG, AZURE_BLOB_NO_CACHE_TEST_CONFIG_YAML]: + with TestConfig(config_str, clazz=UninitializedAzureBlobObjectStore) as (directory, object_store): + assert object_store.cache_size == -1 + assert object_store.staging_path == directory.global_config.object_store_cache_path + + class MockDataset: def __init__(self, id): self.id = id