diff --git a/config/galaxy.yml.sample b/config/galaxy.yml.sample index 3c7335e75f9..408f2574bd5 100644 --- a/config/galaxy.yml.sample +++ b/config/galaxy.yml.sample @@ -528,6 +528,11 @@ galaxy: # it overrides any other objectstore settings. #object_store_config_file: config/object_store_conf.xml + # What Dataset attribute is used to reference files in an ObjectStore + # implementation, default is 'id' but can also be set to 'uuid' for + # more de-centralized usage. + #object_store_store_by: id + # Galaxy sends mail for various things: subscribing users to the # mailing list if they request it, password resets, reporting dataset # errors, and sending activation emails. To do this, it needs to send diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst index 047d5221b21..f5fd69ef56c 100644 --- a/doc/source/admin/galaxy_options.rst +++ b/doc/source/admin/galaxy_options.rst @@ -945,6 +945,18 @@ :Type: str +~~~~~~~~~~~~~~~~~~~~~~~~~ +``object_store_store_by`` +~~~~~~~~~~~~~~~~~~~~~~~~~ + +:Description: + What Dataset attribute is used to reference files in an + ObjectStore implementation, default is 'id' but can also be set to + 'uuid' for more de-centralized usage. +:Default: ``id`` +:Type: str + + ~~~~~~~~~~~~~~~ ``smtp_server`` ~~~~~~~~~~~~~~~ diff --git a/lib/galaxy/config.py b/lib/galaxy/config.py index 7df5462a450..535178b05c0 100644 --- a/lib/galaxy/config.py +++ b/lib/galaxy/config.py @@ -543,6 +543,8 @@ class Configuration(object): self.object_store = kwargs.get('object_store', 'disk') self.object_store_check_old_style = string_as_bool(kwargs.get('object_store_check_old_style', False)) self.object_store_cache_path = resolve_path(kwargs.get("object_store_cache_path", "database/object_store_cache"), self.root) + self.object_store_store_by = kwargs.get("object_store_store_by", "id") + # Handle AWS-specific config options for backward compatibility if kwargs.get('aws_access_key', None) is not None: self.os_access_key = kwargs.get('aws_access_key', None) diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index 732539733f1..803fe133621 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -2003,7 +2003,7 @@ 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_path or "dataset_%d_files" % self.id) + return self.object_store.get_filename(self, dir_only=True, extra_dir=self._extra_files_rel_path) else: return os.path.abspath(self.external_extra_files_path) @@ -2015,7 +2015,12 @@ class Dataset(StorableObject, RepresentById): extra_files_path = property(get_extra_files_path, set_extra_files_path) def extra_files_path_exists(self): - return self.object_store.exists(self, extra_dir=self._extra_files_path or "dataset_%d_files" % self.id, dir_only=True) + return self.object_store.exists(self, extra_dir=self._extra_files_rel_path, dir_only=True) + + @property + def _extra_files_rel_path(self): + store_by = getattr(self.object_store, "store_by", "id") + return self._extra_files_path or "dataset_%s_files" % getattr(self, store_by) def _calculate_size(self): if self.external_filename: @@ -2064,7 +2069,7 @@ class Dataset(StorableObject, RepresentById): if self.file_size is None: self.set_size() self.total_size = self.file_size or 0 - if self.object_store.exists(self, extra_dir=self._extra_files_path or "dataset_%d_files" % self.id, dir_only=True): + if self.object_store.exists(self, extra_dir=self._extra_files_rel_path, dir_only=True): for root, dirs, files in os.walk(self.extra_files_path): self.total_size += sum([os.path.getsize(os.path.join(root, file)) for file in files if os.path.exists(os.path.join(root, file))]) @@ -2090,8 +2095,8 @@ class Dataset(StorableObject, RepresentById): """Remove the file and extra files, marks deleted and purged""" # os.unlink( self.file_name ) self.object_store.delete(self) - if self.object_store.exists(self, extra_dir=self._extra_files_path or "dataset_%d_files" % self.id, dir_only=True): - self.object_store.delete(self, entire_dir=True, extra_dir=self._extra_files_path or "dataset_%d_files" % self.id, dir_only=True) + 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 diff --git a/lib/galaxy/objectstore/__init__.py b/lib/galaxy/objectstore/__init__.py index b3b3504bd65..0bed99b2878 100644 --- a/lib/galaxy/objectstore/__init__.py +++ b/lib/galaxy/objectstore/__init__.py @@ -81,7 +81,7 @@ class ObjectStore(object): 000/obj.id) """ - def __init__(self, config, **kwargs): + def __init__(self, config, config_dict={}, **kwargs): """ :type config: object :param config: An object, most likely populated from @@ -95,11 +95,16 @@ class ObjectStore(object): * new_file_path -- Used to set the 'temp' extra_dir. """ self.running = True - self.extra_dirs = {} self.config = config self.check_old_style = config.object_store_check_old_style - self.extra_dirs['job_work'] = config.jobs_directory - self.extra_dirs['temp'] = config.new_file_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 = {} + extra_dirs['job_work'] = config.jobs_directory + extra_dirs['temp'] = config.new_file_path + extra_dirs.update(dict( + (e['type'], e['path']) for e in config_dict.get('extra_dirs', []))) + self.extra_dirs = extra_dirs def shutdown(self): """Close any connections for this ObjectStore.""" @@ -227,9 +232,13 @@ class ObjectStore(object): return { 'config': config_to_dict(self.config), 'extra_dirs': extra_dirs, + 'store_by': self.store_by, 'type': self.store_type, } + def _get_object_id(self, obj): + return getattr(obj, self.store_by) + class DiskObjectStore(ObjectStore): """ @@ -265,28 +274,21 @@ class DiskObjectStore(ObjectStore): :type extra_dirs: dict :param extra_dirs: Keys are string, values are directory paths. """ - super(DiskObjectStore, self).__init__(config) + super(DiskObjectStore, self).__init__(config, config_dict) self.file_path = config_dict.get("files_dir") or config.file_path - extra_dirs = dict( - (e['type'], e['path']) for e in config_dict.get('extra_dirs', [])) - self.extra_dirs.update(extra_dirs) @classmethod def parse_xml(clazz, config_xml): extra_dirs = [] - file_path = None - + config_dict = {} if config_xml is not None: for e in config_xml: if e.tag == 'files_dir': - file_path = e.get('path') + config_dict["files_dir"] = e.get('path') else: extra_dirs.append({"type": e.get('type'), "path": e.get('path')}) - config_dict = { - "files_dir": file_path, - "extra_dirs": extra_dirs, - } + config_dict["extra_dirs"] = extra_dirs return config_dict def to_dict(self): @@ -359,10 +361,11 @@ class DiskObjectStore(ObjectStore): path = base else: # Construct hashed path - rel_path = os.path.join(*directory_hash_id(obj.id)) + obj_id = self._get_object_id(obj) + rel_path = os.path.join(*directory_hash_id(obj_id)) # Create a subdirectory for the object ID if obj_dir: - rel_path = os.path.join(rel_path, str(obj.id)) + rel_path = os.path.join(rel_path, str(obj_id)) # Optionally append extra_dir if extra_dir is not None: if extra_dir_at_root: @@ -371,7 +374,7 @@ class DiskObjectStore(ObjectStore): rel_path = os.path.join(rel_path, extra_dir) path = os.path.join(base, rel_path) if not dir_only: - path = os.path.join(path, alt_name if alt_name else "dataset_%s.dat" % obj.id) + path = os.path.join(path, alt_name if alt_name else "dataset_%s.dat" % obj_id) return os.path.abspath(path) def exists(self, obj, **kwargs): @@ -559,7 +562,8 @@ class NestedObjectStore(ObjectStore): def _repr_object_for_exception(self, obj): try: # there are a few objects in python that don't have __class__ - return '{}(id={})'.format(obj.__class__.__name__, obj.id) + obj_id = self._get_object_id(obj) + return '{}({}={})'.format(obj.__class__.__name__, self.store_by, obj_id) except AttributeError: return str(obj) @@ -602,7 +606,7 @@ class DistributedObjectStore(NestedObjectStore): :param fsmon: If True, monitor the file system for free space, removing backends when they get too full. """ - super(DistributedObjectStore, self).__init__(config) + super(DistributedObjectStore, self).__init__(config, config_dict) self.backends = {} self.weighted_backend_ids = [] @@ -788,7 +792,7 @@ class HierarchicalObjectStore(NestedObjectStore): def __init__(self, config, config_dict, fsmon=False): """The default contructor. Extends `NestedObjectStore`.""" - super(HierarchicalObjectStore, self).__init__(config) + super(HierarchicalObjectStore, self).__init__(config, config_dict) backends = odict() for order, backend_def in enumerate(config_dict["backends"]): diff --git a/lib/galaxy/objectstore/azure_blob.py b/lib/galaxy/objectstore/azure_blob.py index 412c8403656..80c9fb9d0ce 100644 --- a/lib/galaxy/objectstore/azure_blob.py +++ b/lib/galaxy/objectstore/azure_blob.py @@ -90,7 +90,7 @@ class AzureBlobObjectStore(ObjectStore): store_type = 'azure_blob' def __init__(self, config, config_dict): - super(AzureBlobObjectStore, self).__init__(config) + super(AzureBlobObjectStore, self).__init__(config, config_dict) self.transfer_progress = 0 @@ -107,10 +107,6 @@ class AzureBlobObjectStore(ObjectStore): self.cache_size = cache_dict.get('size', -1) self.staging_path = cache_dict.get('path') or self.config.object_store_cache_path - extra_dirs = dict( - (e['type'], e['path']) for e in config_dict.get('extra_dirs', [])) - self.extra_dirs.update(extra_dirs) - self._initialize() def _initialize(self): diff --git a/lib/galaxy/objectstore/cloud.py b/lib/galaxy/objectstore/cloud.py index 19a536dd84d..3c8a3a15957 100644 --- a/lib/galaxy/objectstore/cloud.py +++ b/lib/galaxy/objectstore/cloud.py @@ -44,7 +44,7 @@ class Cloud(ObjectStore, CloudConfigMixin): store_type = 'cloud' def __init__(self, config, config_dict): - super(Cloud, self).__init__(config) + super(Cloud, self).__init__(config, config_dict) self.transfer_progress = 0 auth_dict = config_dict['auth'] @@ -68,10 +68,6 @@ class Cloud(ObjectStore, CloudConfigMixin): self.cache_size = cache_dict.get('size', -1) self.staging_path = cache_dict.get('path') or self.config.object_store_cache_path - extra_dirs = dict( - (e['type'], e['path']) for e in config_dict.get('extra_dirs', [])) - self.extra_dirs.update(extra_dirs) - self._initialize() def _initialize(self): diff --git a/lib/galaxy/objectstore/pithos.py b/lib/galaxy/objectstore/pithos.py index 414279fc04b..297f06bd8e2 100644 --- a/lib/galaxy/objectstore/pithos.py +++ b/lib/galaxy/objectstore/pithos.py @@ -87,16 +87,12 @@ class PithosObjectStore(ObjectStore): store_type = 'pithos' def __init__(self, config, config_dict): - super(PithosObjectStore, self).__init__(config) + super(PithosObjectStore, self).__init__(config, config_dict) self.staging_path = self.config.file_path log.info('Parse config_xml for pithos object store') self.config_dict = config_dict log.debug(self.config_dict) - log.info('Define extra_dirs') - extra_dirs = dict( - (e['type'], e['path']) for e in config_dict.get('extra_dirs', [])) - self.extra_dirs.update(extra_dirs) self._initialize() def _initialize(self): diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml index 62593a88b7c..0743565fcbd 100644 --- a/lib/galaxy/webapps/galaxy/config_schema.yml +++ b/lib/galaxy/webapps/galaxy/config_schema.yml @@ -720,6 +720,14 @@ mapping: Configuration file for the object store If this is set and exists, it overrides any other objectstore settings. + object_store_store_by: + type: str + default: id + required: false + desc: | + What Dataset attribute is used to reference files in an ObjectStore implementation, + default is 'id' but can also be set to 'uuid' for more de-centralized usage. + smtp_server: type: str default: '' diff --git a/test/unit/test_objectstore.py b/test/unit/test_objectstore.py index 3be1aeeaefa..34e06245197 100644 --- a/test/unit/test_objectstore.py +++ b/test/unit/test_objectstore.py @@ -3,6 +3,7 @@ from contextlib import contextmanager from shutil import rmtree from string import Template from tempfile import mkdtemp +from uuid import uuid4 from xml.etree import ElementTree import yaml @@ -14,6 +15,7 @@ from galaxy.objectstore.azure_blob import AzureBlobObjectStore from galaxy.objectstore.cloud import Cloud from galaxy.objectstore.pithos import PithosObjectStore from galaxy.objectstore.s3 import S3ObjectStore +from galaxy.util import directory_hash_id DISK_TEST_CONFIG = """ @@ -93,6 +95,75 @@ def test_disk_store(): assert not os.path.exists(to_delete_real_path) +DISK_TEST_CONFIG_BY_UUID_YAML = """ +type: disk +files_dir: "${temp_directory}/files1" +store_by: uuid +extra_dirs: + - type: temp + path: "${temp_directory}/tmp1" + - type: job_work + path: "${temp_directory}/job_working_directory1" +""" + + +def test_disk_store_by_uuid(): + for config_str in [DISK_TEST_CONFIG_BY_UUID_YAML]: + with TestConfig(config_str) as (directory, object_store): + # Test no dataset with id 1 exists. + absent_dataset = MockDataset(1) + assert not object_store.exists(absent_dataset) + + # Write empty dataset 2 in second backend, ensure it is empty and + # exists. + empty_dataset = MockDataset(2) + directory.write("", "files1/%s/dataset_%s.dat" % (empty_dataset.rel_path_for_uuid_test(), empty_dataset.uuid)) + assert object_store.exists(empty_dataset) + assert object_store.empty(empty_dataset) + + # Write non-empty dataset in backend 1, test it is not emtpy & exists. + hello_world_dataset = MockDataset(3) + directory.write("Hello World!", "files1/%s/dataset_%s.dat" % (hello_world_dataset.rel_path_for_uuid_test(), hello_world_dataset.uuid)) + assert object_store.exists(hello_world_dataset) + assert not object_store.empty(hello_world_dataset) + + # Test get_data + data = object_store.get_data(hello_world_dataset) + assert data == "Hello World!" + + data = object_store.get_data(hello_world_dataset, start=1, count=6) + assert data == "ello W" + + # Test Size + + # Test absent and empty datasets yield size of 0. + assert object_store.size(absent_dataset) == 0 + assert object_store.size(empty_dataset) == 0 + # Elsewise + assert object_store.size(hello_world_dataset) > 0 # Should this always be the number of bytes? + + # Test percent used (to some degree) + percent_store_used = object_store.get_store_usage_percent() + assert percent_store_used > 0.0 + assert percent_store_used < 100.0 + + # Test update_from_file test + output_dataset = MockDataset(4) + output_real_path = os.path.join(directory.temp_directory, "files1", output_dataset.rel_path_for_uuid_test(), "dataset_%s.dat" % output_dataset.uuid) + assert not os.path.exists(output_real_path) + output_working_path = directory.write("NEW CONTENTS", "job_working_directory1/example_output") + object_store.update_from_file(output_dataset, file_name=output_working_path, create=True) + assert os.path.exists(output_real_path) + + # Test delete + to_delete_dataset = MockDataset(5) + to_delete_real_path = directory.write("content to be deleted!", "files1/%s/dataset_%s.dat" % (to_delete_dataset.rel_path_for_uuid_test(), to_delete_dataset.uuid)) + assert object_store.exists(to_delete_dataset) + assert object_store.delete(to_delete_dataset) + assert not object_store.exists(to_delete_dataset) + assert not os.path.exists(to_delete_real_path) + + def test_disk_store_alt_name_relpath(): """ Test that alt_name cannot be used to access arbitrary paths using a relative path @@ -636,8 +707,13 @@ class MockDataset(object): def __init__(self, id): self.id = id self.object_store_id = None + self.uuid = uuid4() self.tags = [] + def rel_path_for_uuid_test(self): + rel_path = os.path.join(*directory_hash_id(self.uuid)) + return rel_path + # Poor man's mocking. Need to get a real mocking library as real Galaxy development # dependnecy.