From 0aafc6e1592c9a4769507c07d3098484bd9ba079 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Fri, 28 Feb 2020 16:17:13 -0500 Subject: [PATCH 01/18] Login is its own entrypoint and we don't want it shoved into an iframe anymore. --- client/galaxy/scripts/layout/menu.js | 1 - 1 file changed, 1 deletion(-) diff --git a/client/galaxy/scripts/layout/menu.js b/client/galaxy/scripts/layout/menu.js index 46c1ead9f75..ff5c15ec448 100644 --- a/client/galaxy/scripts/layout/menu.js +++ b/client/galaxy/scripts/layout/menu.js @@ -253,7 +253,6 @@ const Collection = Backbone.Collection.extend({ cls: "loggedout-only", tooltip: _l("Login"), url: "login", - target: "galaxy_main", noscratchbook: true }; } From a8cbd36d9049f4cd8c86bf12cb1790a16d2304a4 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 29 Feb 2020 17:09:26 +0100 Subject: [PATCH 02/18] Drop manage_dependency_relationships altogether Probably didn't work for some time and seems unnecessary. Fixes ``` galaxy.tool_shed.galaxy_install.installed_repository_manager DEBUG 2020-02-28 10:57:01,109 [p:94029,w:0,m:0] [MainThread] Adding an entry for version 2.2.4 of package bowtie2 to runtime_tool_dependencies_of_installed_tool_dependencies. Traceback (most recent call last): File "lib/galaxy/webapps/galaxy/buildapp.py", line 48, in app_factory app = galaxy.app.UniverseApplication(global_conf=global_conf, **kwargs) File "lib/galaxy/app.py", line 95, in __init__ self.installed_repository_manager = InstalledRepositoryManager(self) File "lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py", line 84, in __init__ self.load_dependency_relationships() File "lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py", line 734, in load_dependency_relationships self.add_entry_to_runtime_tool_dependencies_of_installed_tool_dependencies(tool_dependency) File "lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py", line 218, in add_entry_to_runtime_tool_dependencies_of_installed_tool_dependencies status=None) File "lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py", line 604, in get_runtime_dependent_tool_dependency_tuples required_env_shell_file_path = tool_dependency.get_env_shell_file_path(self.app) File "lib/galaxy/model/tool_shed_install/__init__.py", line 522, in get_env_shell_file_path installation_directory = self.installation_directory(app) File "lib/galaxy/model/tool_shed_install/__init__.py", line 534, in installation_directory return os.path.join(app.tool_dependency_dir, File "lib/galaxy/config/__init__.py", line 1186, in tool_dependency_dir return self.toolbox.dependency_manager.default_base_path AttributeError: 'UniverseApplication' object has no attribute 'toolbox' ``` reported by @scholtalbers --- doc/source/admin/galaxy_options.rst | 13 --- lib/galaxy/app.py | 2 - lib/galaxy/config/sample/galaxy.yml.sample | 6 -- .../galaxy_install/install_manager.py | 8 -- .../installed_repository_manager.py | 91 ------------------- lib/galaxy/webapps/galaxy/config_schema.yml | 9 -- 6 files changed, 129 deletions(-) diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst index f4948b3c19e..619dd9b285f 100644 --- a/doc/source/admin/galaxy_options.rst +++ b/doc/source/admin/galaxy_options.rst @@ -697,19 +697,6 @@ :Type: int -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -``manage_dependency_relationships`` -~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - -:Description: - Enable use of an in-memory registry with bi-directional - relationships between repositories (i.e., in addition to lists of - dependencies for a repository, keep an in-memory registry of - dependent items for each repository. -:Default: ``false`` -:Type: bool - - ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ``tool_data_table_config_path`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/lib/galaxy/app.py b/lib/galaxy/app.py index 7cc31f01f08..5b13f02a54b 100644 --- a/lib/galaxy/app.py +++ b/lib/galaxy/app.py @@ -91,9 +91,7 @@ class UniverseApplication(config.ConfiguresGalaxyMixin): check_migrate_tools = self.config.check_migrate_tools self._configure_models(check_migrate_databases=self.config.check_migrate_databases, check_migrate_tools=check_migrate_tools, config_file=config_file) - # Manage installed tool shed repositories. self.installed_repository_manager = InstalledRepositoryManager(self) - self._configure_datatypes_registry(self.installed_repository_manager) galaxy.model.set_datatypes_registry(self.datatypes_registry) diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample index 1263f0d1b1f..acd9c1bd0a0 100644 --- a/lib/galaxy/config/sample/galaxy.yml.sample +++ b/lib/galaxy/config/sample/galaxy.yml.sample @@ -447,12 +447,6 @@ galaxy: # between 1 and 24. #hours_between_check: 12 - # Enable use of an in-memory registry with bi-directional - # relationships between repositories (i.e., in addition to lists of - # dependencies for a repository, keep an in-memory registry of - # dependent items for each repository. - #manage_dependency_relationships: false - # XML config file that contains data table entries for the # ToolDataTableManager. This file is manually # maintained by the # Galaxy administrator (.sample used if default does not exist). diff --git a/lib/galaxy/tool_shed/galaxy_install/install_manager.py b/lib/galaxy/tool_shed/galaxy_install/install_manager.py index 3a2b2bedfc2..687aa9bf964 100644 --- a/lib/galaxy/tool_shed/galaxy_install/install_manager.py +++ b/lib/galaxy/tool_shed/galaxy_install/install_manager.py @@ -215,10 +215,6 @@ class InstallToolDependencyManager(object): if tool_dependency and tool_dependency.status in [self.install_model.ToolDependency.installation_status.INSTALLED, self.install_model.ToolDependency.installation_status.ERROR]: installed_packages.append(tool_dependency) - if self.app.config.manage_dependency_relationships: - # Add the tool_dependency to the in-memory dictionaries in the installed_repository_manager. - self.app.installed_repository_manager.handle_tool_dependency_install(tool_shed_repository, - tool_dependency) return installed_packages def install_via_fabric(self, tool_shed_repository, tool_dependency, install_dir, package_name=None, custom_fabfile_path=None, @@ -946,10 +942,6 @@ class InstallRepositoryManager(object): basic_util.remove_dir(work_dir) self.update_tool_shed_repository_status(tool_shed_repository, self.install_model.ToolShedRepository.installation_status.INSTALLED) - if self.app.config.manage_dependency_relationships: - # Add the installed repository and any tool dependencies to the in-memory dictionaries - # in the installed_repository_manager. - self.app.installed_repository_manager.handle_repository_install(tool_shed_repository) else: # An error occurred while cloning the repository, so reset everything necessary to enable another attempt. repository_util.set_repository_attributes(self.app, diff --git a/lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py b/lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py index 58f3ba686d6..3ccd62febb7 100644 --- a/lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py +++ b/lib/galaxy/tool_shed/galaxy_install/installed_repository_manager.py @@ -79,9 +79,6 @@ class InstalledRepositoryManager(object): # whose values are a list of tuples defining tool_dependency objects (whose status is 'Installed') that require the key # at runtime. The value defines the entire tool dependency tree. self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies = {} - if app.config.manage_dependency_relationships: - # Load defined dependency relationships for installed tool shed repositories and their contents. - self.load_dependency_relationships() def activate_repository(self, repository): """Activate an installed tool shed repository that has been marked as deactivated.""" @@ -651,90 +648,6 @@ class InstalledRepositoryManager(object): deleted_tool_dependency_names.append(original_dependency_val_dict['name']) return updated_tool_dependency_names, deleted_tool_dependency_names - def handle_repository_install(self, repository): - """Load the dependency relationships for a repository that was just installed or reinstalled.""" - # Populate self.repository_dependencies_of_installed_repositories. - self.add_entry_to_repository_dependencies_of_installed_repositories(repository) - # Populate self.installed_repository_dependencies_of_installed_repositories. - self.add_entry_to_installed_repository_dependencies_of_installed_repositories(repository) - # Populate self.tool_dependencies_of_installed_repositories. - self.add_entry_to_tool_dependencies_of_installed_repositories(repository) - # Populate self.installed_tool_dependencies_of_installed_repositories. - self.add_entry_to_installed_tool_dependencies_of_installed_repositories(repository) - for tool_dependency in repository.tool_dependencies: - # Populate self.runtime_tool_dependencies_of_installed_tool_dependencies. - self.add_entry_to_runtime_tool_dependencies_of_installed_tool_dependencies(tool_dependency) - # Populate self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies. - self.add_entry_to_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies(tool_dependency) - - def handle_repository_uninstall(self, repository): - """Remove the dependency relationships for a repository that was just uninstalled.""" - for tool_dependency in repository.tool_dependencies: - tool_dependency_tup = self.get_tool_dependency_tuple_for_installed_repository_manager(tool_dependency) - # Remove this tool_dependency from all values in - # self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies - altered_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies = {} - for (td_tup, installed_runtime_dependent_tool_dependency_tups) in self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies.items(): - if tool_dependency_tup in installed_runtime_dependent_tool_dependency_tups: - # Remove the tool_dependency from the list. - installed_runtime_dependent_tool_dependency_tups.remove(tool_dependency_tup) - # Add the possibly altered list to the altered dictionary. - altered_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies[td_tup] = \ - installed_runtime_dependent_tool_dependency_tups - self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies = \ - altered_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies - # Remove the entry for this tool_dependency from self.runtime_tool_dependencies_of_installed_tool_dependencies. - self.remove_entry_from_runtime_tool_dependencies_of_installed_tool_dependencies(tool_dependency) - # Remove the entry for this tool_dependency from - # self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies. - self.remove_entry_from_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies(tool_dependency) - # Remove this repository's entry from self.installed_tool_dependencies_of_installed_repositories. - self.remove_entry_from_installed_tool_dependencies_of_installed_repositories(repository) - # Remove this repository's entry from self.tool_dependencies_of_installed_repositories - self.remove_entry_from_tool_dependencies_of_installed_repositories(repository) - # Remove this repository's entry from self.installed_repository_dependencies_of_installed_repositories. - self.remove_entry_from_installed_repository_dependencies_of_installed_repositories(repository) - # Remove this repository's entry from self.repository_dependencies_of_installed_repositories. - self.remove_entry_from_repository_dependencies_of_installed_repositories(repository) - - def handle_tool_dependency_install(self, repository, tool_dependency): - """Load the dependency relationships for a tool dependency that was just installed independently of its containing repository.""" - # The received repository must have a status of 'Installed'. The value of tool_dependency.status will either be - # 'Installed' or 'Error', but we only need to change the in-memory dictionaries if it is 'Installed'. - if tool_dependency.is_installed: - # Populate self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies. - self.add_entry_to_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies(tool_dependency) - # Populate self.installed_tool_dependencies_of_installed_repositories. - repository_tup = self.get_repository_tuple_for_installed_repository_manager(repository) - tool_dependency_tup = self.get_tool_dependency_tuple_for_installed_repository_manager(tool_dependency) - if repository_tup in self.installed_tool_dependencies_of_installed_repositories: - self.installed_tool_dependencies_of_installed_repositories[repository_tup].append(tool_dependency_tup) - else: - self.installed_tool_dependencies_of_installed_repositories[repository_tup] = [tool_dependency_tup] - - def load_dependency_relationships(self): - """Load relationships for all installed repositories and tool dependencies into in-memnory dictionaries.""" - # Get the list of installed tool shed repositories. - for repository in self.context.query(self.app.install_model.ToolShedRepository) \ - .filter(self.app.install_model.ToolShedRepository.table.c.status == - self.app.install_model.ToolShedRepository.installation_status.INSTALLED): - # Populate self.repository_dependencies_of_installed_repositories. - self.add_entry_to_repository_dependencies_of_installed_repositories(repository) - # Populate self.installed_repository_dependencies_of_installed_repositories. - self.add_entry_to_installed_repository_dependencies_of_installed_repositories(repository) - # Populate self.tool_dependencies_of_installed_repositories. - self.add_entry_to_tool_dependencies_of_installed_repositories(repository) - # Populate self.installed_tool_dependencies_of_installed_repositories. - self.add_entry_to_installed_tool_dependencies_of_installed_repositories(repository) - # Get the list of installed tool dependencies. - for tool_dependency in self.context.query(self.app.install_model.ToolDependency) \ - .filter(self.app.install_model.ToolDependency.table.c.status == - self.app.install_model.ToolDependency.installation_status.INSTALLED): - # Populate self.runtime_tool_dependencies_of_installed_tool_dependencies. - self.add_entry_to_runtime_tool_dependencies_of_installed_tool_dependencies(tool_dependency) - # Populate self.installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies. - self.add_entry_to_installed_runtime_dependent_tool_dependencies_of_installed_tool_dependencies(tool_dependency) - def load_proprietary_datatypes(self): cdl = custom_datatype_manager.CustomDatatypeLoader(self.app) for tool_shed_repository in self.context.query(self.install_model.ToolShedRepository) \ @@ -814,10 +727,6 @@ class InstalledRepositoryManager(object): if remove_from_disk: repository.status = self.app.install_model.ToolShedRepository.installation_status.UNINSTALLED repository.error_message = None - if self.app.config.manage_dependency_relationships: - # Remove the uninstalled repository and any tool dependencies from the in-memory dictionaries in the - # installed_repository_manager. - self.handle_repository_uninstall(repository) else: repository.status = self.app.install_model.ToolShedRepository.installation_status.DEACTIVATED self.app.install_model.context.current.add(repository) diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml index a723a9aec95..d8101b9a342 100644 --- a/lib/galaxy/webapps/galaxy/config_schema.yml +++ b/lib/galaxy/webapps/galaxy/config_schema.yml @@ -528,15 +528,6 @@ mapping: server process should be able to check for repository updates. The setting for hours_between_check should be an integer between 1 and 24. - manage_dependency_relationships: - type: bool - default: false - required: false - desc: | - Enable use of an in-memory registry with bi-directional relationships between - repositories (i.e., in addition to lists of dependencies for a repository, - keep an in-memory registry of dependent items for each repository. - tool_data_table_config_path: type: str default: config/tool_data_table_conf.xml From 37dfd4292a69e4f76dcec785361649956eb1c8ca Mon Sep 17 00:00:00 2001 From: Helena Rasche Date: Fri, 1 Feb 2019 18:45:52 +0100 Subject: [PATCH 03/18] 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 04/18] 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 05/18] 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 06/18] 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 07/18] 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 08/18] 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 09/18] 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 10/18] 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 11/18] 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 04db689fd0ffc71e491c4404759b27ba9ba0d4b1 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 3 Mar 2020 08:27:21 -0500 Subject: [PATCH 12/18] Fix default quota creation message. --- lib/galaxy/actions/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/actions/admin.py b/lib/galaxy/actions/admin.py index 3879f6b794e..d2acc040db9 100644 --- a/lib/galaxy/actions/admin.py +++ b/lib/galaxy/actions/admin.py @@ -45,7 +45,7 @@ class AdminActions(object): # If this is a default quota, create the DefaultQuotaAssociation if params.default != 'no': self.app.quota_agent.set_default_quota(params.default, quota) - message = "Default quota '%s' has been created." + message = "Default quota '%s' has been created." % quota.name else: # Create the UserQuotaAssociations in_users = [self.sa_session.query(self.app.model.User).get(decode_id(x) if decode_id else x) for x in util.listify(params.in_users)] From 46f3af60450098a5a7846edb2f1fec31873cc7d4 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 15:20:17 +0100 Subject: [PATCH 13/18] 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 f54b0e729a5fa8d223df03f824e767ca413b0127 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 3 Mar 2020 10:02:19 -0500 Subject: [PATCH 14/18] Fix data manager table slot usage; this makes the header with the name and 'reload' icon display again. Also refactor a few computed props. Should add this to eslint to catch these easier. --- .../admin/DataManager/DataManagerTable.vue | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue b/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue index fa1139df378..f749b464514 100644 --- a/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue +++ b/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue @@ -13,22 +13,24 @@ - - - - - - - - - {{ this.dataTable["name"] }} - - - + { if (response.data.dataTable) { this.dataTable = response.data.dataTable; From ac9249eb4e4e0f6a5d67b870fa62eccedbe2c7f3 Mon Sep 17 00:00:00 2001 From: Dannon Baker Date: Tue, 3 Mar 2020 10:04:46 -0500 Subject: [PATCH 15/18] Prettier. --- .../components/admin/DataManager/DataManagerTable.vue | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue b/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue index f749b464514..972612a8116 100644 --- a/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue +++ b/client/galaxy/scripts/components/admin/DataManager/DataManagerTable.vue @@ -17,11 +17,7 @@ - + @@ -71,7 +67,7 @@ export default { }, computed: { dataTableName() { - return this.dataTable && this.dataTable.name ? this.dataTable.name : 'null'; + return this.dataTable && this.dataTable.name ? this.dataTable.name : "null"; }, buttonLabel() { return `Reload ${this.dataTableName} tool data table`; From 494c15cb9d622c3c75bbcf72b880ee386cbe01f5 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 3 Mar 2020 18:31:06 +0100 Subject: [PATCH 16/18] 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 17/18] 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 18/18] 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):