From 454b6b7c7af43d37e7844c7d37d694eb8d748d96 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 12 Apr 2020 16:13:51 +0200 Subject: [PATCH 01/33] Use beaker cache for etree documents and profile create_tool --- lib/galaxy/tools/__init__.py | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index f19b350272f..e504043ce96 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -20,6 +20,8 @@ from xml.etree import ElementTree import packaging.version import webob.exc +from beaker.cache import CacheManager +from beaker.util import parse_cache_config_options from mako.template import Template from six import itervalues, string_types from six.moves.urllib.parse import unquote_plus @@ -114,6 +116,8 @@ from .execute import ( MappingParameters, ) +import profilehooks + log = logging.getLogger(__name__) REQUIRES_JS_RUNTIME_MESSAGE = ("The tool [%s] requires a nodejs runtime to execute " @@ -244,6 +248,12 @@ class ToolBox(BaseGalaxyToolBox): def __init__(self, config_filenames, tool_root_dir, app, save_integrated_tool_panel=True): self._reload_count = 0 self.tool_location_fetcher = ToolLocationFetcher() + cache_opts = { + 'cache.type': getattr(app.config, 'tool_cache_type', 'dbm'), + 'cache.data_dir': getattr(app.config, 'tool_cache_data_dir', 'database/tool_cache/data'), + 'cache.lock_dir': getattr(app.config, 'tool_cache_lock_dir', 'database/tool_cache/lock'), + } + self.expanded_tool_source_cache = CacheManager(**parse_cache_config_options(cache_opts)).get_cache('tool_source') # This is here to deal with the old default value, which doesn't make # sense in an "installed Galaxy" world. # FIXME: ./ @@ -282,17 +292,22 @@ class ToolBox(BaseGalaxyToolBox): # Deprecated method, TODO - eliminate calls to this in test/. return self._tools_by_id + @profilehooks.profile def create_tool(self, config_file, **kwds): - try: - tool_source = get_tool_source( - config_file, - enable_beta_formats=getattr(self.app.config, "enable_beta_tool_formats", False), - tool_location_fetcher=self.tool_location_fetcher, - ) - except Exception as e: - # capture and log parsing errors - global_tool_errors.add_error(config_file, "Tool XML parsing", e) - raise e + + def get_expanded_tool_source(): + try: + return get_tool_source( + config_file, + enable_beta_formats=getattr(self.app.config, "enable_beta_tool_formats", False), + tool_location_fetcher=self.tool_location_fetcher, + ) + except Exception as e: + # capture and log parsing errors + global_tool_errors.add_error(config_file, "Tool XML parsing", e) + raise e + + tool_source = self.expanded_tool_source_cache.get(key=config_file, createfunc=get_expanded_tool_source) return self._create_tool_from_source(tool_source, config_file=config_file, **kwds) def _create_tool_from_source(self, tool_source, **kwds): From 8d0c5c81d0cb4657d0ec9b11b3f921cd3bfa89b8 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 13 Apr 2020 12:18:38 +0200 Subject: [PATCH 02/33] Use dogpile cache + XML serialization of expanded tools + skip read locks --- lib/galaxy/tools/__init__.py | 106 +++++++++++++++++++++++++++---- lib/galaxy/tools/toolbox/base.py | 2 + 2 files changed, 94 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index e504043ce96..4f5ad327654 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -117,6 +117,84 @@ from .execute import ( ) import profilehooks +from dogpile.cache import make_region +from dogpile.cache.api import ( + NO_VALUE, + CachedValue, +) +from dogpile.cache.proxy import ProxyBackend +from dogpile.util import ReadWriteMutex +from dogpile.cache.backends.file import AbstractFileLock + +from xml.etree import ElementTree +from lxml import etree + + +class XmlBackend(ProxyBackend): + + def set(self, key, value): + with self.proxied._dbm_file(True) as dbm: + dbm[key] = json.dumps({'metadata': value.metadata, 'payload': self.value_encode(value)}) + + def get(self, key): + with self.proxied._dbm_file(False) as dbm: + if hasattr(dbm, "get"): + value = dbm.get(key, NO_VALUE) + else: + # gdbm objects lack a .get method + try: + value = dbm[key] + except KeyError: + value = NO_VALUE + if value is not NO_VALUE: + value = self.value_decode(value) + return value + + def value_decode(self, v): + if not v or v is NO_VALUE: + return NO_VALUE + # you probably want to specify a custom decoder via `object_hook` + v = json.loads(v) + payload = get_tool_source(xml_tree=ElementTree.ElementTree(etree.fromstring(v['payload'].encode('utf-8')))) + return CachedValue(metadata=v['metadata'], payload=payload) + + def value_encode(self, v): + # you probably want to specify a custom encoder via `default` + payload = ElementTree.tostring(v.payload.root, encoding='utf8', method='xml').decode('utf-8') + return payload + +class MutexLock(AbstractFileLock): + def __init__(self, filename): + self.mutex = ReadWriteMutex() + + def acquire_read_lock(self, wait): + return True + ret = self.mutex.acquire_read_lock(wait) + return wait or ret + + def acquire_write_lock(self, wait): + ret = self.mutex.acquire_write_lock(wait) + return wait or ret + + def release_read_lock(self): + return True + return self.mutex.release_read_lock() + + def release_write_lock(self): + return self.mutex.release_write_lock() + + +region = make_region().configure( + 'dogpile.cache.dbm', + arguments={ + "filename": "database/tool_cache/cache.dbm", + "lock_factory": MutexLock, + }, + expiration_time=-1, + wrap=[XmlBackend], +) + + log = logging.getLogger(__name__) @@ -292,24 +370,24 @@ class ToolBox(BaseGalaxyToolBox): # Deprecated method, TODO - eliminate calls to this in test/. return self._tools_by_id - @profilehooks.profile def create_tool(self, config_file, **kwds): - def get_expanded_tool_source(): - try: - return get_tool_source( - config_file, - enable_beta_formats=getattr(self.app.config, "enable_beta_tool_formats", False), - tool_location_fetcher=self.tool_location_fetcher, - ) - except Exception as e: - # capture and log parsing errors - global_tool_errors.add_error(config_file, "Tool XML parsing", e) - raise e - - tool_source = self.expanded_tool_source_cache.get(key=config_file, createfunc=get_expanded_tool_source) + tool_source = self.get_expanded_tool_source(config_file) return self._create_tool_from_source(tool_source, config_file=config_file, **kwds) + @region.cache_on_arguments() + def get_expanded_tool_source(self, config_file): + try: + return get_tool_source( + config_file, + enable_beta_formats=getattr(self.app.config, "enable_beta_tool_formats", False), + tool_location_fetcher=self.tool_location_fetcher, + ) + except Exception as e: + # capture and log parsing errors + global_tool_errors.add_error(config_file, "Tool XML parsing", e) + raise e + def _create_tool_from_source(self, tool_source, **kwds): return create_tool_from_source(self.app, tool_source, **kwds) diff --git a/lib/galaxy/tools/toolbox/base.py b/lib/galaxy/tools/toolbox/base.py index 4794e20d779..d6fade8fd28 100644 --- a/lib/galaxy/tools/toolbox/base.py +++ b/lib/galaxy/tools/toolbox/base.py @@ -47,6 +47,8 @@ from .tags import tool_tag_manager log = logging.getLogger(__name__) +import profilehooks + SHED_TOOL_CONF_XML = """ From 484654ce1323fed76ecf4b017935e101d868af4e Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 13 Apr 2020 17:04:32 +0200 Subject: [PATCH 03/33] Use lxml for tool document parsing speedup lxml is considerably faster (about 3 seconds for main's toolbox). lxml tracks membership automatically, if you append an element to a paremt the element will be removed from the child. --- lib/galaxy/tool_util/parser/xml.py | 4 ---- lib/galaxy/tools/__init__.py | 4 ++-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index 110524b4659..7f9a4af5c1a 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -711,21 +711,18 @@ def __expand_input_elems(root_elem, prefix=""): new_prefix = __prefix_join(prefix, name, index=index) __expand_input_elems(repeat_elem, new_prefix) __pull_up_params(root_elem, repeat_elem) - root_elem.remove(repeat_elem) cond_elems = root_elem.findall('conditional') for cond_elem in cond_elems: new_prefix = __prefix_join(prefix, cond_elem.get("name")) __expand_input_elems(cond_elem, new_prefix) __pull_up_params(root_elem, cond_elem) - root_elem.remove(cond_elem) section_elems = root_elem.findall('section') for section_elem in section_elems: new_prefix = __prefix_join(prefix, section_elem.get("name")) __expand_input_elems(section_elem, new_prefix) __pull_up_params(root_elem, section_elem) - root_elem.remove(section_elem) def __append_prefix_to_params(elem, prefix): @@ -736,7 +733,6 @@ def __append_prefix_to_params(elem, prefix): def __pull_up_params(parent_elem, child_elem): for param_elem in child_elem.findall('param'): parent_elem.append(param_elem) - child_elem.remove(param_elem) def __prefix_join(prefix, name, index=None): diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 4f5ad327654..5a787253536 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -155,12 +155,12 @@ class XmlBackend(ProxyBackend): return NO_VALUE # you probably want to specify a custom decoder via `object_hook` v = json.loads(v) - payload = get_tool_source(xml_tree=ElementTree.ElementTree(etree.fromstring(v['payload'].encode('utf-8')))) + payload = get_tool_source(xml_tree=etree.ElementTree(etree.fromstring(v['payload'].encode('utf-8')))) return CachedValue(metadata=v['metadata'], payload=payload) def value_encode(self, v): # you probably want to specify a custom encoder via `default` - payload = ElementTree.tostring(v.payload.root, encoding='utf8', method='xml').decode('utf-8') + payload = etree.tounicode(v.payload.root, encoding='utf8', method='xml') return payload class MutexLock(AbstractFileLock): From 4a1d87fcb229202eac387fa085b5721abe792c0b Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 13 Apr 2020 17:05:31 +0200 Subject: [PATCH 04/33] Delay parsing of tool inputs and outputs until they are needed --- lib/galaxy/tools/__init__.py | 29 ++++++++++++++++++++--------- lib/galaxy/workflow/modules.py | 8 +++++--- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 5a787253536..f63c77f6612 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -585,6 +585,9 @@ class Tool(Dictifiable): self.populate_resource_parameters(tool_source) self.tool_errors = None # Parse XML element containing configuration + self.tool_source = tool_source + self._is_workflow_compatible = None + self.finalized = False try: self.parse(tool_source, guid=guid, dynamic=dynamic) except Exception as e: @@ -595,6 +598,12 @@ class Tool(Dictifiable): if self.app.name == 'galaxy': self.job_search = JobSearch(app=self.app) + def assert_finalized(self): + if self.finalized is False: + self.parse_inputs(self.tool_source) + self.parse_outputs(self.tool_source) + self.finalized = True + @property def history_manager(self): return self.app.history_manager @@ -896,15 +905,9 @@ class Tool(Dictifiable): self.provided_metadata_file = tool_source.parse_provided_metadata_file() self.provided_metadata_style = tool_source.parse_provided_metadata_style() - # Parse tool inputs (if there are any required) - self.parse_inputs(tool_source) - # Parse tool help self.parse_help(tool_source) - # Description of outputs produced by an invocation of the tool - self.parse_outputs(tool_source) - # Parse result handling for tool exit codes and stdout/stderr messages: self.parse_stdio(tool_source) @@ -937,8 +940,6 @@ class Tool(Dictifiable): self.citations = self._parse_citations(tool_source) self.xrefs = tool_source.parse_xrefs() - # Determine if this tool can be used in workflows - self.is_workflow_compatible = self.check_workflow_compatible(tool_source) self.__parse_trackster_conf(tool_source) # Record macro paths so we can reload a tool if any of its macro has changes self._macro_paths = tool_source.macro_paths() @@ -1445,6 +1446,15 @@ class Tool(Dictifiable): else: return self.outputs.get(name, None) + @property + def is_workflow_compatible(self): + is_workflow_compatible = self._is_workflow_compatible + if is_workflow_compatible is None: + is_workflow_compatible = self.check_workflow_compatible(self.tool_source) + if self.finalized: + self._is_workflow_compatible = is_workflow_compatible + return is_workflow_compatible + def check_workflow_compatible(self, tool_source): """ Determine if a tool can be used in workflows. External tools and the @@ -1452,7 +1462,7 @@ class Tool(Dictifiable): """ # Multiple page tools are not supported -- we're eliminating most # of these anyway - if self.has_multiple_pages: + if self.finalized and self.has_multiple_pages: return False # This is probably the best bet for detecting external web tools # right now @@ -2079,6 +2089,7 @@ class Tool(Dictifiable): """ history_id = kwd.get('history_id', None) history = None + self.assert_finalized() if workflow_building_mode is workflow_building_modes.USE_HISTORY or workflow_building_mode is workflow_building_modes.DISABLED: # We don't need a history when exporting a workflow for the workflow editor or when downloading a workflow try: diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 31a2348c926..55c7ce242cc 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -1233,9 +1233,11 @@ class ToolModule(WorkflowModule): self.tool_version = tool_version self.tool_uuid = tool_uuid self.tool = trans.app.toolbox.get_tool(tool_id, tool_version=tool_version, exact=exact_tools, tool_uuid=tool_uuid) - if self.tool and tool_version and exact_tools and str(self.tool.version) != str(tool_version): - log.info("Exact tool specified during workflow module creation for [%s] but couldn't find correct version [%s]." % (tool_id, tool_version)) - self.tool = None + if self.tool: + if tool_version and exact_tools and str(self.tool.version) != str(tool_version): + log.info("Exact tool specified during workflow module creation for [%s] but couldn't find correct version [%s]." % (tool_id, tool_version)) + self.tool = None + self.tool.assert_finalized() self.post_job_actions = {} self.runtime_post_job_actions = {} self.workflow_outputs = [] From 51549d0391c4e651322ff418122fd50f3a25c8a4 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 13 Apr 2020 18:21:49 +0200 Subject: [PATCH 05/33] Only build toolbox index for webapps, delay until after startup --- lib/galaxy/app.py | 10 +++++++++- lib/galaxy/config/__init__.py | 1 - lib/galaxy/queue_worker.py | 7 +++++-- lib/galaxy/web/framework/webapp.py | 1 + lib/galaxy/webapps/galaxy/buildapp.py | 1 - lib/tool_shed/webapp/app.py | 2 ++ scripts/galaxy-main | 1 - 7 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/app.py b/lib/galaxy/app.py index 5b13f02a54b..4a7e3c01904 100644 --- a/lib/galaxy/app.py +++ b/lib/galaxy/app.py @@ -24,7 +24,10 @@ from galaxy.managers.users import UserManager from galaxy.managers.workflows import WorkflowsManager from galaxy.model.database_heartbeat import DatabaseHeartbeat from galaxy.model.tags import GalaxyTagHandler -from galaxy.queue_worker import GalaxyQueueWorker +from galaxy.queue_worker import ( + send_local_control_task, + GalaxyQueueWorker, +) from galaxy.tool_shed.galaxy_install.installed_repository_manager import InstalledRepositoryManager from galaxy.tool_shed.galaxy_install.update_repository_manager import UpdateRepositoryManager from galaxy.tool_util.deps.views import DependencyResolversView @@ -65,6 +68,8 @@ class UniverseApplication(config.ConfiguresGalaxyMixin): logging.basicConfig(level=logging.DEBUG) log.debug("python path is: %s", ", ".join(sys.path)) self.name = 'galaxy' + # is_webapp will be set to true when building WSGI app + self.is_webapp = False self.startup_timer = ExecutionTimer() self.new_installation = False # Read config file and check for errors @@ -244,6 +249,9 @@ class UniverseApplication(config.ConfiguresGalaxyMixin): # Start web stack message handling self.application_stack.register_postfork_function(self.application_stack.start) + self.application_stack.register_postfork_function(self.queue_worker.bind_and_start) + # Delay toolbox index until after startup + self.application_stack.register_postfork_function(lambda: send_local_control_task(self, 'rebuild_toolbox_search_index')) self.model.engine.dispose() diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py index bc28f5e0e01..e0412f5b188 100644 --- a/lib/galaxy/config/__init__.py +++ b/lib/galaxy/config/__init__.py @@ -1032,7 +1032,6 @@ class ConfiguresGalaxyMixin(object): self._set_enabled_container_types() index_help = getattr(self.config, "index_tool_help", True) self.toolbox_search = galaxy.tools.search.ToolBoxSearch(self.toolbox, index_help) - self.reindex_tool_search() def reindex_tool_search(self): # Call this when tools are added or removed. diff --git a/lib/galaxy/queue_worker.py b/lib/galaxy/queue_worker.py index df671acdfb9..3589ef63ed7 100644 --- a/lib/galaxy/queue_worker.py +++ b/lib/galaxy/queue_worker.py @@ -247,8 +247,11 @@ def reload_tool_data_tables(app, **kwargs): def rebuild_toolbox_search_index(app, **kwargs): - if app.toolbox_search.index_count < app.toolbox._reload_count: - app.reindex_tool_search() + if app.is_webapp: + if app.toolbox_search.index_count < app.toolbox._reload_count: + app.reindex_tool_search() + else: + log.debug("App is not a webapp, not building a search index") def reload_job_rules(app, **kwargs): diff --git a/lib/galaxy/web/framework/webapp.py b/lib/galaxy/web/framework/webapp.py index 6447ea08bd6..03b69af8bca 100644 --- a/lib/galaxy/web/framework/webapp.py +++ b/lib/galaxy/web/framework/webapp.py @@ -79,6 +79,7 @@ class WebApplication(base.WebApplication): def __init__(self, galaxy_app, session_cookie='galaxysession', name=None): self.name = name base.WebApplication.__init__(self) + galaxy_app.is_webapp = True self.set_transaction_factory(lambda e: self.transaction_chooser(e, galaxy_app, session_cookie)) # Mako support self.mako_template_lookup = self.create_mako_template_lookup(galaxy_app, name) diff --git a/lib/galaxy/webapps/galaxy/buildapp.py b/lib/galaxy/webapps/galaxy/buildapp.py index 9df4eb0ad50..8a6c0e643fe 100644 --- a/lib/galaxy/webapps/galaxy/buildapp.py +++ b/lib/galaxy/webapps/galaxy/buildapp.py @@ -204,7 +204,6 @@ uwsgi_app_factory = uwsgi_app def postfork_setup(): from galaxy.app import app - app.queue_worker.bind_and_start() app.application_stack.log_startup() diff --git a/lib/tool_shed/webapp/app.py b/lib/tool_shed/webapp/app.py index 77db4e3e236..b917d6700a4 100644 --- a/lib/tool_shed/webapp/app.py +++ b/lib/tool_shed/webapp/app.py @@ -27,6 +27,8 @@ class UniverseApplication(object): def __init__(self, **kwd): log.debug("python path is: %s", ", ".join(sys.path)) self.name = "tool_shed" + # will be overwritten when building WSGI app + self.is_webapp = False # Read the tool_shed.ini configuration file and check for errors. self.config = config.Configuration(**kwd) self.config.check() diff --git a/scripts/galaxy-main b/scripts/galaxy-main index 470d34502a0..08720d6dee0 100755 --- a/scripts/galaxy-main +++ b/scripts/galaxy-main @@ -108,7 +108,6 @@ def load_galaxy_app( **kwds ) app.database_heartbeat.start() - app.queue_worker.bind_and_start() app.application_stack.log_startup() return app From c6bd67ed870d3d8675872ee73467fbac9b6ed941 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 13 Apr 2020 21:45:06 +0200 Subject: [PATCH 06/33] Drop profilehook import, minor cleanup --- lib/galaxy/tools/__init__.py | 10 ++++------ lib/galaxy/tools/toolbox/base.py | 2 -- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index f63c77f6612..e817116316e 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -116,7 +116,6 @@ from .execute import ( MappingParameters, ) -import profilehooks from dogpile.cache import make_region from dogpile.cache.api import ( NO_VALUE, @@ -126,7 +125,6 @@ from dogpile.cache.proxy import ProxyBackend from dogpile.util import ReadWriteMutex from dogpile.cache.backends.file import AbstractFileLock -from xml.etree import ElementTree from lxml import etree @@ -163,14 +161,16 @@ class XmlBackend(ProxyBackend): payload = etree.tounicode(v.payload.root, encoding='utf8', method='xml') return payload + class MutexLock(AbstractFileLock): def __init__(self, filename): self.mutex = ReadWriteMutex() def acquire_read_lock(self, wait): + # No need for read lock. It is supposed to prevent the "dogpile" effect + # where multiple functions each create the cached resource, but I don't + # think we care. return True - ret = self.mutex.acquire_read_lock(wait) - return wait or ret def acquire_write_lock(self, wait): ret = self.mutex.acquire_write_lock(wait) @@ -178,7 +178,6 @@ class MutexLock(AbstractFileLock): def release_read_lock(self): return True - return self.mutex.release_read_lock() def release_write_lock(self): return self.mutex.release_write_lock() @@ -195,7 +194,6 @@ region = make_region().configure( ) - log = logging.getLogger(__name__) REQUIRES_JS_RUNTIME_MESSAGE = ("The tool [%s] requires a nodejs runtime to execute " diff --git a/lib/galaxy/tools/toolbox/base.py b/lib/galaxy/tools/toolbox/base.py index d6fade8fd28..4794e20d779 100644 --- a/lib/galaxy/tools/toolbox/base.py +++ b/lib/galaxy/tools/toolbox/base.py @@ -47,8 +47,6 @@ from .tags import tool_tag_manager log = logging.getLogger(__name__) -import profilehooks - SHED_TOOL_CONF_XML = """ From 540f532bdd648f9cd8d47665cbf6d2a4aaca2730 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 13 Apr 2020 22:20:46 +0200 Subject: [PATCH 07/33] Store macro_path --- lib/galaxy/tool_util/parser/factory.py | 4 ++-- lib/galaxy/tools/__init__.py | 8 +++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/tool_util/parser/factory.py b/lib/galaxy/tool_util/parser/factory.py index c4fb95029e4..ab2bc86c81b 100644 --- a/lib/galaxy/tool_util/parser/factory.py +++ b/lib/galaxy/tool_util/parser/factory.py @@ -14,14 +14,14 @@ from ..fetcher import ToolLocationFetcher log = logging.getLogger(__name__) -def get_tool_source(config_file=None, xml_tree=None, enable_beta_formats=True, tool_location_fetcher=None): +def get_tool_source(config_file=None, xml_tree=None, enable_beta_formats=True, tool_location_fetcher=None, macro_paths=None): """Return a ToolSource object corresponding to supplied source. The supplied source may be specified as a file path (using the config_file parameter) or as an XML object loaded with load_tool_with_refereces. """ if xml_tree is not None: - return XmlToolSource(xml_tree, source_path=config_file) + return XmlToolSource(xml_tree, source_path=config_file, macro_paths=macro_paths) elif config_file is None: raise ValueError("get_tool_source called with invalid config_file None.") diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index e817116316e..c21b05c1d5a 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -132,7 +132,7 @@ class XmlBackend(ProxyBackend): def set(self, key, value): with self.proxied._dbm_file(True) as dbm: - dbm[key] = json.dumps({'metadata': value.metadata, 'payload': self.value_encode(value)}) + dbm[key] = json.dumps({'metadata': value.metadata, 'payload': self.value_encode(value), 'macro_paths': value.payload.macro_paths()}) def get(self, key): with self.proxied._dbm_file(False) as dbm: @@ -151,14 +151,12 @@ class XmlBackend(ProxyBackend): def value_decode(self, v): if not v or v is NO_VALUE: return NO_VALUE - # you probably want to specify a custom decoder via `object_hook` v = json.loads(v) - payload = get_tool_source(xml_tree=etree.ElementTree(etree.fromstring(v['payload'].encode('utf-8')))) + payload = get_tool_source(xml_tree=etree.ElementTree(etree.fromstring(v['payload'].encode('utf-8'))), macro_paths=v['macro_paths']) return CachedValue(metadata=v['metadata'], payload=payload) def value_encode(self, v): - # you probably want to specify a custom encoder via `default` - payload = etree.tounicode(v.payload.root, encoding='utf8', method='xml') + payload = ElementTree.tostring(v.payload.root, encoding="utf-8", method='xml').decode('utf-8') return payload From bea3d0e41f8835945f8cc7a3e624dbf4ed004ecd Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 13 Apr 2020 22:30:54 +0200 Subject: [PATCH 08/33] Fix imports --- lib/galaxy/app.py | 2 +- lib/galaxy/tools/__init__.py | 24 +++++++++++------------- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/lib/galaxy/app.py b/lib/galaxy/app.py index 4a7e3c01904..2ff73b060ed 100644 --- a/lib/galaxy/app.py +++ b/lib/galaxy/app.py @@ -25,8 +25,8 @@ from galaxy.managers.workflows import WorkflowsManager from galaxy.model.database_heartbeat import DatabaseHeartbeat from galaxy.model.tags import GalaxyTagHandler from galaxy.queue_worker import ( - send_local_control_task, GalaxyQueueWorker, + send_local_control_task, ) from galaxy.tool_shed.galaxy_install.installed_repository_manager import InstalledRepositoryManager from galaxy.tool_shed.galaxy_install.update_repository_manager import UpdateRepositoryManager diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index c21b05c1d5a..e1ec27ae763 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -22,6 +22,15 @@ import packaging.version import webob.exc from beaker.cache import CacheManager from beaker.util import parse_cache_config_options +from dogpile.cache import make_region +from dogpile.cache.api import ( + CachedValue, + NO_VALUE, +) +from dogpile.cache.backends.file import AbstractFileLock +from dogpile.cache.proxy import ProxyBackend +from dogpile.util import ReadWriteMutex +from lxml import etree from mako.template import Template from six import itervalues, string_types from six.moves.urllib.parse import unquote_plus @@ -116,19 +125,8 @@ from .execute import ( MappingParameters, ) -from dogpile.cache import make_region -from dogpile.cache.api import ( - NO_VALUE, - CachedValue, -) -from dogpile.cache.proxy import ProxyBackend -from dogpile.util import ReadWriteMutex -from dogpile.cache.backends.file import AbstractFileLock -from lxml import etree - - -class XmlBackend(ProxyBackend): +class JSONBackend(ProxyBackend): def set(self, key, value): with self.proxied._dbm_file(True) as dbm: @@ -188,7 +186,7 @@ region = make_region().configure( "lock_factory": MutexLock, }, expiration_time=-1, - wrap=[XmlBackend], + wrap=[JSONBackend], ) From 785cfce3fae70a40bb287656151fcdf687515809 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 13 Apr 2020 23:03:22 +0200 Subject: [PATCH 09/33] Fix unit tests --- lib/galaxy/tool_shed/tools/tool_validator.py | 1 + test/unit/tools_support.py | 1 + test/unit/workflows/test_modules.py | 3 ++- 3 files changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/tool_shed/tools/tool_validator.py b/lib/galaxy/tool_shed/tools/tool_validator.py index e98ae396679..495bb277d0f 100644 --- a/lib/galaxy/tool_shed/tools/tool_validator.py +++ b/lib/galaxy/tool_shed/tools/tool_validator.py @@ -78,6 +78,7 @@ class ToolValidator(object): ) try: tool = create_tool_from_source(config_file=full_path, app=self.app, tool_source=tool_source, repository_id=repository_id, allow_code_files=False) + tool.assert_finalized() valid = True error_message = None except KeyError as e: diff --git a/test/unit/tools_support.py b/test/unit/tools_support.py index 1724558830b..665b2d4cb4d 100644 --- a/test/unit/tools_support.py +++ b/test/unit/tools_support.py @@ -99,6 +99,7 @@ class UsesTools(object): tool_source = get_tool_source(self.tool_file) try: self.tool = create_tool_from_source(self.app, tool_source, config_file=self.tool_file) + self.tool.assert_finalized() except Exception: self.tool = None if getattr(self, "tool_action", None and self.tool): diff --git a/test/unit/workflows/test_modules.py b/test/unit/workflows/test_modules.py index 8327493a6d7..5c42e2d374e 100644 --- a/test/unit/workflows/test_modules.py +++ b/test/unit/workflows/test_modules.py @@ -456,7 +456,8 @@ def __mock_tool( output_type='data')}, params_from_strings=mock.Mock(), check_and_update_param_values=mock.Mock(), - to_json=_to_json + to_json=_to_json, + assert_finalized=lambda: None, ) return tool From 642744d897c8edb1163728b9bfdb0bb977943c53 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 13 Apr 2020 23:24:13 +0200 Subject: [PATCH 10/33] Make tool cache data dir configurable --- doc/source/admin/galaxy_options.rst | 11 +++++++++ lib/galaxy/config/sample/galaxy.yml.sample | 4 ++++ lib/galaxy/tools/__init__.py | 26 +++++++++------------ lib/galaxy/webapps/galaxy/config_schema.yml | 8 +++++++ 4 files changed, 34 insertions(+), 15 deletions(-) diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst index e4b98522e38..514c3036b68 100644 --- a/doc/source/admin/galaxy_options.rst +++ b/doc/source/admin/galaxy_options.rst @@ -1002,6 +1002,17 @@ :Type: str +~~~~~~~~~~~~~~~~~~~~~~~ +``tool_cache_data_dir`` +~~~~~~~~~~~~~~~~~~~~~~~ + +:Description: + Tool related caching. Full expanded tools and metadata wll be + stroed at this path. +:Default: ``tool_cache`` +:Type: str + + ~~~~~~~~~~~~~~~~~~~~~~~ ``citation_cache_type`` ~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample index 1c644e43c4f..8950280601e 100644 --- a/lib/galaxy/config/sample/galaxy.yml.sample +++ b/lib/galaxy/config/sample/galaxy.yml.sample @@ -588,6 +588,10 @@ galaxy: # generated commands run in sh. #default_job_shell: /bin/bash + # Tool related caching. Full expanded tools and metadata wll be stroed + # at this path. + #tool_cache_data_dir: tool_cache + # Citation related caching. Tool citations information maybe fetched # from external sources such as https://doi.org/ by Galaxy - the # following parameters can be used to control the caching used to diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index e1ec27ae763..c798e8e957e 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -179,15 +179,7 @@ class MutexLock(AbstractFileLock): return self.mutex.release_write_lock() -region = make_region().configure( - 'dogpile.cache.dbm', - arguments={ - "filename": "database/tool_cache/cache.dbm", - "lock_factory": MutexLock, - }, - expiration_time=-1, - wrap=[JSONBackend], -) +region = make_region() log = logging.getLogger(__name__) @@ -320,12 +312,16 @@ class ToolBox(BaseGalaxyToolBox): def __init__(self, config_filenames, tool_root_dir, app, save_integrated_tool_panel=True): self._reload_count = 0 self.tool_location_fetcher = ToolLocationFetcher() - cache_opts = { - 'cache.type': getattr(app.config, 'tool_cache_type', 'dbm'), - 'cache.data_dir': getattr(app.config, 'tool_cache_data_dir', 'database/tool_cache/data'), - 'cache.lock_dir': getattr(app.config, 'tool_cache_lock_dir', 'database/tool_cache/lock'), - } - self.expanded_tool_source_cache = CacheManager(**parse_cache_config_options(cache_opts)).get_cache('tool_source') + os.makedirs(app.config.tool_cache_data_dir, exist_ok=True) + region.configure( + 'dogpile.cache.dbm', + arguments={ + "filename": os.path.join(app.config.tool_cache_data_dir, "cache.dbm"), + "lock_factory": MutexLock, + }, + expiration_time=-1, + wrap=[JSONBackend], + ) # This is here to deal with the old default value, which doesn't make # sense in an "installed Galaxy" world. # FIXME: ./ diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml index 29dc0bf9675..cbdcfd0cda5 100644 --- a/lib/galaxy/webapps/galaxy/config_schema.yml +++ b/lib/galaxy/webapps/galaxy/config_schema.yml @@ -749,6 +749,14 @@ mapping: should be disabled. Containerized jobs always use /bin/sh - so more maximum portability tool authors should assume generated commands run in sh. + tool_cache_data_dir: + type: str + default: tool_cache + path_resolves_to: data_dir + required: false + desc: | + Tool related caching. Full expanded tools and metadata wll be stored at this path. + citation_cache_type: type: str default: file From 7c3335496b820bba19d5c709970b8a903127204d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 13 Apr 2020 23:29:33 +0200 Subject: [PATCH 11/33] Make sure tools are fully initialized before executing --- lib/galaxy/tools/__init__.py | 3 +++ lib/galaxy/tools/evaluation.py | 1 + 2 files changed, 4 insertions(+) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index c798e8e957e..6fb77a92d58 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -321,6 +321,7 @@ class ToolBox(BaseGalaxyToolBox): }, expiration_time=-1, wrap=[JSONBackend], + replace_existing_backend=True, ) # This is here to deal with the old default value, which doesn't make # sense in an "installed Galaxy" world. @@ -1510,6 +1511,7 @@ class Tool(Dictifiable): visit_input_values(self.inputs, values, callback) def expand_incoming(self, trans, incoming, request_context): + self.assert_finalized() rerun_remap_job_id = None if 'rerun_remap_job_id' in incoming: try: @@ -1702,6 +1704,7 @@ class Tool(Dictifiable): `self.tool_action`. In general this will create a `Job` that when run will build the tool's outputs, e.g. `DefaultToolAction`. """ + self.assert_finalized() try: return self.tool_action.execute(self, trans, incoming=incoming, set_output_hid=set_output_hid, history=history, **kwargs) except exceptions.ToolExecutionError as exc: diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index 8ba4fe676a9..4adb64401f5 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -57,6 +57,7 @@ class ToolEvaluator(object): def __init__(self, app, tool, job, local_working_directory): self.app = app self.job = job + tool.assert_finalized() self.tool = tool self.local_working_directory = local_working_directory From 1a0f451fa25f1e28bc291e749c461824a3e25352 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 14 Apr 2020 09:53:42 +0200 Subject: [PATCH 12/33] Use __getattr__ to establish inputs/outputs --- lib/galaxy/tools/__init__.py | 27 ++++++++++++++++++++++++--- lib/galaxy/tools/evaluation.py | 1 - lib/galaxy/workflow/modules.py | 1 - 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 6fb77a92d58..2e5c17c0c2c 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -589,6 +589,29 @@ class Tool(Dictifiable): if self.app.name == 'galaxy': self.job_search = JobSearch(app=self.app) + def __getattr__(self, name): + lazy_attributes = { + 'action', + 'check_values', + 'display_by_page', + 'enctype', + 'has_multiple_pages', + 'inputs', + 'inputs_by_page', + 'last_page', + 'method', + 'npages', + 'nginx_upload', + 'target', + 'template_macro_params', + 'outputs', + 'output_collections' + } + if name in lazy_attributes: + self.assert_finalized() + return getattr(self, name) + raise AttributeError(name) + def assert_finalized(self): if self.finalized is False: self.parse_inputs(self.tool_source) @@ -1015,6 +1038,7 @@ class Tool(Dictifiable): @property def tests(self): + self.assert_finalized() if not self.__tests_populated: tests_source = self.__tests_source if tests_source: @@ -1511,7 +1535,6 @@ class Tool(Dictifiable): visit_input_values(self.inputs, values, callback) def expand_incoming(self, trans, incoming, request_context): - self.assert_finalized() rerun_remap_job_id = None if 'rerun_remap_job_id' in incoming: try: @@ -1704,7 +1727,6 @@ class Tool(Dictifiable): `self.tool_action`. In general this will create a `Job` that when run will build the tool's outputs, e.g. `DefaultToolAction`. """ - self.assert_finalized() try: return self.tool_action.execute(self, trans, incoming=incoming, set_output_hid=set_output_hid, history=history, **kwargs) except exceptions.ToolExecutionError as exc: @@ -2082,7 +2104,6 @@ class Tool(Dictifiable): """ history_id = kwd.get('history_id', None) history = None - self.assert_finalized() if workflow_building_mode is workflow_building_modes.USE_HISTORY or workflow_building_mode is workflow_building_modes.DISABLED: # We don't need a history when exporting a workflow for the workflow editor or when downloading a workflow try: diff --git a/lib/galaxy/tools/evaluation.py b/lib/galaxy/tools/evaluation.py index 4adb64401f5..8ba4fe676a9 100644 --- a/lib/galaxy/tools/evaluation.py +++ b/lib/galaxy/tools/evaluation.py @@ -57,7 +57,6 @@ class ToolEvaluator(object): def __init__(self, app, tool, job, local_working_directory): self.app = app self.job = job - tool.assert_finalized() self.tool = tool self.local_working_directory = local_working_directory diff --git a/lib/galaxy/workflow/modules.py b/lib/galaxy/workflow/modules.py index 55c7ce242cc..800620fe770 100644 --- a/lib/galaxy/workflow/modules.py +++ b/lib/galaxy/workflow/modules.py @@ -1237,7 +1237,6 @@ class ToolModule(WorkflowModule): if tool_version and exact_tools and str(self.tool.version) != str(tool_version): log.info("Exact tool specified during workflow module creation for [%s] but couldn't find correct version [%s]." % (tool_id, tool_version)) self.tool = None - self.tool.assert_finalized() self.post_job_actions = {} self.runtime_post_job_actions = {} self.workflow_outputs = [] From 5ecec9514a7c4f46fa2af1008ef39d9c1567daf9 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 14 Apr 2020 12:12:55 +0200 Subject: [PATCH 13/33] ToolShed app really doesn't need toolbox --- lib/galaxy/tools/toolbox/base.py | 9 ++------- lib/tool_shed/webapp/app.py | 2 -- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/tools/toolbox/base.py b/lib/galaxy/tools/toolbox/base.py index 4794e20d779..a70f75c40a5 100644 --- a/lib/galaxy/tools/toolbox/base.py +++ b/lib/galaxy/tools/toolbox/base.py @@ -106,13 +106,8 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): # (e.g., shed_tool_conf.xml) files include the tool_path attribute within the tag. self._tool_root_dir = tool_root_dir self.app = app - if hasattr(self.app, 'watchers'): - self._tool_watcher = self.app.watchers.tool_watcher - self._tool_config_watcher = self.app.watchers.tool_config_watcher - else: - # Toolbox is loaded but not used during toolshed tests - self._tool_watcher = None - self._tool_config_watcher = None + self._tool_watcher = self.app.watchers.tool_watcher + self._tool_config_watcher = self.app.watchers.tool_config_watcher self._filter_factory = FilterFactory(self) self._tool_tag_manager = tool_tag_manager(app) self._init_tools_from_configs(config_filenames) diff --git a/lib/tool_shed/webapp/app.py b/lib/tool_shed/webapp/app.py index b917d6700a4..de82ab53f3f 100644 --- a/lib/tool_shed/webapp/app.py +++ b/lib/tool_shed/webapp/app.py @@ -66,9 +66,7 @@ class UniverseApplication(object): # Citation manager needed to load tools. from galaxy.managers.citations import CitationsManager self.citations_manager = CitationsManager(self) - # The Tool Shed makes no use of a Galaxy toolbox, but this attribute is still required. self.use_tool_dependency_resolution = False - self.toolbox = tools.ToolBox([], self.config.tool_path, self) # Initialize the Tool Shed security agent. self.security_agent = self.model.security_agent # The Tool Shed makes no use of a quota, but this attribute is still required. From f60b3dc07c7071e69467380306485262df4a10d4 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 14 Apr 2020 12:20:46 +0200 Subject: [PATCH 14/33] Expire cached tool source --- lib/galaxy/tools/cache.py | 2 ++ test/unit/unittest_utils/galaxy_mock.py | 1 + 2 files changed, 3 insertions(+) diff --git a/lib/galaxy/tools/cache.py b/lib/galaxy/tools/cache.py index 36ce418ab75..24f1934cf22 100644 --- a/lib/galaxy/tools/cache.py +++ b/lib/galaxy/tools/cache.py @@ -8,6 +8,7 @@ from sqlalchemy.orm import ( joinedload, ) +from galaxy.tools import region from galaxy.util import unicodify from galaxy.util.hash_util import md5_hash_file @@ -42,6 +43,7 @@ class ToolCache(object): with self._lock: paths_to_cleanup = {path: tool.all_ids for path, tool in self._tools_by_path.items() if self._should_cleanup(path)} for config_filename, tool_ids in paths_to_cleanup.items(): + region.delete(config_filename) del self._hash_by_tool_paths[config_filename] if os.path.exists(config_filename): # This tool has probably been broken while editing on disk diff --git a/test/unit/unittest_utils/galaxy_mock.py b/test/unit/unittest_utils/galaxy_mock.py index 82ac950bdb4..b579fcd09aa 100644 --- a/test/unit/unittest_utils/galaxy_mock.py +++ b/test/unit/unittest_utils/galaxy_mock.py @@ -170,6 +170,7 @@ class MockAppConfig(Bunch): # set by MockDir self.root = root + self.tool_cache_data_dir = os.path.join(root, 'tool_cache') self.config_file = None From 015dcd07bd1a1b6bae8c748cd2b7de44a45d8c23 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 14 Apr 2020 12:37:12 +0200 Subject: [PATCH 15/33] Add dogpile cache expiration, fixes reload on tool/macro change --- lib/galaxy/tools/__init__.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 2e5c17c0c2c..78e40fe3abe 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -20,8 +20,6 @@ from xml.etree import ElementTree import packaging.version import webob.exc -from beaker.cache import CacheManager -from beaker.util import parse_cache_config_options from dogpile.cache import make_region from dogpile.cache.api import ( CachedValue, @@ -179,8 +177,14 @@ class MutexLock(AbstractFileLock): return self.mutex.release_write_lock() -region = make_region() +def my_key_generator(namespace, fn, **kw): + def generate_key(*arg): + return "_".join(str(s) for s in arg) + + return generate_key + +region = make_region(function_key_generator=my_key_generator) log = logging.getLogger(__name__) From f4d4496075b87770bc3be0c0ea3216c5fe571acb Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 14 Apr 2020 12:39:04 +0200 Subject: [PATCH 16/33] Lint fixes --- lib/galaxy/tools/__init__.py | 1 + lib/tool_shed/webapp/app.py | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 78e40fe3abe..61ae5234015 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -184,6 +184,7 @@ def my_key_generator(namespace, fn, **kw): return generate_key + region = make_region(function_key_generator=my_key_generator) log = logging.getLogger(__name__) diff --git a/lib/tool_shed/webapp/app.py b/lib/tool_shed/webapp/app.py index de82ab53f3f..8b5ce91cc99 100644 --- a/lib/tool_shed/webapp/app.py +++ b/lib/tool_shed/webapp/app.py @@ -8,7 +8,6 @@ import galaxy.tools.data import tool_shed.repository_registry import tool_shed.repository_types.registry import tool_shed.webapp.model -from galaxy import tools from galaxy.config import configure_logging from galaxy.model.tags import CommunityTagHandler from galaxy.security import idencoding From 8ca6288a3a70eb31d0d898c2e83751c454c4df51 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 14 Apr 2020 12:49:51 +0200 Subject: [PATCH 17/33] Tweak key function --- lib/galaxy/tools/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 61ae5234015..6e537aa299f 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -180,7 +180,7 @@ class MutexLock(AbstractFileLock): def my_key_generator(namespace, fn, **kw): def generate_key(*arg): - return "_".join(str(s) for s in arg) + return "_".join(str(s) for s in arg if isinstance(s, str)) return generate_key From 2102d2c93c550df0ffe8f720f19ddf41fde8ed69 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 14 Apr 2020 13:20:26 +0200 Subject: [PATCH 18/33] Setup inputs in parse_inputs --- lib/galaxy/tools/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 6e537aa299f..a6fe9e8c1a3 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -536,7 +536,6 @@ class Tool(Dictifiable): self.repository_id = repository_id self._allow_code_files = allow_code_files # setup initial attribute values - self.inputs = OrderedDict() self.stdio_exit_codes = list() self.stdio_regexes = list() self.inputs_by_page = list() @@ -1129,6 +1128,7 @@ class Tool(Dictifiable): This implementation supports multiple pages and grouping constructs. """ # Load parameters (optional) + self.inputs = OrderedDict() pages = tool_source.parse_input_pages() enctypes = set() if pages.inputs_defined: From b67429b9b7f6a1abb29fed37d9cb1a0c42cc1b1d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 14 Apr 2020 14:12:50 +0200 Subject: [PATCH 19/33] Legacy python fixes --- lib/galaxy/tools/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index a6fe9e8c1a3..5ff3432d88b 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -147,7 +147,8 @@ class JSONBackend(ProxyBackend): def value_decode(self, v): if not v or v is NO_VALUE: return NO_VALUE - v = json.loads(v) + # v is returned as bytestring, so we need to `unicodify` on python < 3.6 before we can use json.loads + v = json.loads(unicodify(v)) payload = get_tool_source(xml_tree=etree.ElementTree(etree.fromstring(v['payload'].encode('utf-8'))), macro_paths=v['macro_paths']) return CachedValue(metadata=v['metadata'], payload=payload) @@ -317,7 +318,8 @@ class ToolBox(BaseGalaxyToolBox): def __init__(self, config_filenames, tool_root_dir, app, save_integrated_tool_panel=True): self._reload_count = 0 self.tool_location_fetcher = ToolLocationFetcher() - os.makedirs(app.config.tool_cache_data_dir, exist_ok=True) + if not os.path.exists(app.config.tool_cache_data_dir): + os.makedirs(app.config.tool_cache_data_dir) region.configure( 'dogpile.cache.dbm', arguments={ From 81fc3c2cc6f4694e0c21615c64851fb9c9ba0b07 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Tue, 14 Apr 2020 14:41:00 +0200 Subject: [PATCH 20/33] Use CSafeLoader if possible --- lib/galaxy/util/yaml_util.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/util/yaml_util.py b/lib/galaxy/util/yaml_util.py index bb466a96981..95718627de3 100644 --- a/lib/galaxy/util/yaml_util.py +++ b/lib/galaxy/util/yaml_util.py @@ -5,13 +5,17 @@ import os from collections import OrderedDict import yaml +try: + from yaml import CSafeLoader as SafeLoader +except ImportError: + from yaml import SafeLoader from yaml.constructor import ConstructorError log = logging.getLogger(__name__) -class OrderedLoader(yaml.SafeLoader): +class OrderedLoader(SafeLoader): # This class was pulled out of ordered_load() for the sake of # mocking __init__ in a unit test. def __init__(self, stream): From 56e2257a011803075bee72737019b63613c1a7be Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 15 Apr 2020 20:49:25 +0200 Subject: [PATCH 21/33] Calculate cached tool hash in tool watcher thread --- lib/galaxy/tools/cache.py | 43 +++++++++++++++++++++-------- lib/galaxy/tools/toolbox/watcher.py | 2 ++ 2 files changed, 34 insertions(+), 11 deletions(-) diff --git a/lib/galaxy/tools/cache.py b/lib/galaxy/tools/cache.py index 24f1934cf22..59ee6b1d1f5 100644 --- a/lib/galaxy/tools/cache.py +++ b/lib/galaxy/tools/cache.py @@ -27,10 +27,16 @@ class ToolCache(object): self._tools_by_path = {} self._tool_paths_by_id = {} self._macro_paths_by_id = {} - self._mod_time_by_path = {} self._new_tool_ids = set() self._removed_tool_ids = set() self._removed_tools_by_path = {} + self._hashes_initialized = False + + def assert_hashes_initialized(self): + if not self._hashes_initialized: + for tool_hash in self._hash_by_tool_paths.values(): + tool_hash.hash + self._hashes_initialized = True def cleanup(self): """ @@ -72,13 +78,14 @@ class ToolCache(object): if not os.path.exists(config_filename): return True new_mtime = os.path.getmtime(config_filename) - if self._mod_time_by_path.get(config_filename) < new_mtime: - if md5_hash_file(config_filename) != self._hash_by_tool_paths.get(config_filename): + tool_hash = self._hash_by_tool_paths.get(config_filename) + if tool_hash.modtime < new_mtime: + if md5_hash_file(config_filename) != tool_hash.hash: return True tool = self._tools_by_path[config_filename] for macro_path in tool._macro_paths: new_mtime = os.path.getmtime(macro_path) - if self._mod_time_by_path.get(macro_path) < new_mtime: + if self._hash_by_tool_paths.get(macro_path).modtime < new_mtime: return True return False @@ -100,23 +107,21 @@ class ToolCache(object): del self._hash_by_tool_paths[config_filename] del self._tool_paths_by_id[tool_id] del self._tools_by_path[config_filename] - del self._mod_time_by_path[config_filename] if tool_id in self._new_tool_ids: self._new_tool_ids.remove(tool_id) def cache_tool(self, config_filename, tool): - tool_hash = md5_hash_file(config_filename) - if tool_hash is None: - return tool_id = str(tool.id) + # We defer hashing of the config file if we haven't called assert_hashes_initialized. + # This allows startup to occur without having to read in and hash all tool and macro files + lazy_hash = not self._hashes_initialized with self._lock: - self._hash_by_tool_paths[config_filename] = tool_hash - self._mod_time_by_path[config_filename] = os.path.getmtime(config_filename) + self._hash_by_tool_paths[config_filename] = ToolHash(config_filename, lazy_hash=lazy_hash) self._tool_paths_by_id[tool_id] = config_filename self._tools_by_path[config_filename] = tool self._new_tool_ids.add(tool_id) for macro_path in tool._macro_paths: - self._mod_time_by_path[macro_path] = os.path.getmtime(macro_path) + self._hash_by_tool_paths[macro_path] = ToolHash(macro_path, lazy_hash=lazy_hash) if tool_id not in self._macro_paths_by_id: self._macro_paths_by_id[tool_id] = {macro_path} else: @@ -132,6 +137,22 @@ class ToolCache(object): self._removed_tools_by_path = {} +class ToolHash(object): + + def __init__(self, path, modtime=None, lazy_hash=False): + self.path = path + self.modtime = modtime or os.path.getmtime(path) + self._tool_hash = None + if not lazy_hash: + self.hash + + @property + def hash(self): + if self._tool_hash is None: + self._tool_hash = md5_hash_file(self.path) + return self._tool_hash + + class ToolShedRepositoryCache(object): """ Cache installed ToolShedRepository objects. diff --git a/lib/galaxy/tools/toolbox/watcher.py b/lib/galaxy/tools/toolbox/watcher.py index 7733ded05ab..68d36886f5f 100644 --- a/lib/galaxy/tools/toolbox/watcher.py +++ b/lib/galaxy/tools/toolbox/watcher.py @@ -99,6 +99,8 @@ class ToolConfWatcher(object): def check(self): """Check for changes in self.paths or self.cache and call the event handler.""" hashes = {} + if self.cache: + self.cache.assert_hashes_initialized() while self._active and not self.exit.isSet(): do_reload = False drop_on_next_loop = set() From 14b5b4f8ee517e164c384bf551763a41d19ac3b2 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 15 Apr 2020 20:46:18 +0200 Subject: [PATCH 22/33] Keep tool lineage versions sorted That is more efficient than sorting every time we access tool_versions. --- lib/galaxy/dependencies/pipfiles/default/Pipfile | 1 + .../pipfiles/default/pinned-requirements.txt | 1 + lib/galaxy/tools/__init__.py | 2 +- lib/galaxy/tools/toolbox/lineages/interface.py | 9 +++------ 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/galaxy/dependencies/pipfiles/default/Pipfile b/lib/galaxy/dependencies/pipfiles/default/Pipfile index 0b669e32aaa..2a56fbeb780 100644 --- a/lib/galaxy/dependencies/pipfiles/default/Pipfile +++ b/lib/galaxy/dependencies/pipfiles/default/Pipfile @@ -62,6 +62,7 @@ dictobj = "*" nose = "*" Parsley = "*" six = "*" +sortedcontainers = "*" Whoosh = "*" galaxy_sequence_utils = "*" "h5py" = "!=2.7.0, !=2.7.1" diff --git a/lib/galaxy/dependencies/pipfiles/default/pinned-requirements.txt b/lib/galaxy/dependencies/pipfiles/default/pinned-requirements.txt index 948a30e797e..80b4f37d751 100644 --- a/lib/galaxy/dependencies/pipfiles/default/pinned-requirements.txt +++ b/lib/galaxy/dependencies/pipfiles/default/pinned-requirements.txt @@ -171,6 +171,7 @@ shellescape==3.4.1 simplejson==3.17.0 six==1.11.0 social-auth-core[openidconnect]==3.3.0 +sortedcontainers==2.1.0 sqlalchemy-migrate==0.13.0 sqlalchemy-utils==0.36.3 sqlalchemy==1.3.16 diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 5ff3432d88b..9c73f26caf0 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -650,7 +650,7 @@ class Tool(Dictifiable): def tool_versions(self): # If we have versions, return them. if self.lineage: - return self.lineage.tool_versions + return list(self.lineage.tool_versions) else: return [] diff --git a/lib/galaxy/tools/toolbox/lineages/interface.py b/lib/galaxy/tools/toolbox/lineages/interface.py index bfc8167ed53..19e66f286bf 100644 --- a/lib/galaxy/tools/toolbox/lineages/interface.py +++ b/lib/galaxy/tools/toolbox/lineages/interface.py @@ -1,6 +1,7 @@ import threading import packaging.version +from sortedcontainers import SortedSet from galaxy.util.tool_version import remove_version_from_guid @@ -39,11 +40,7 @@ class ToolLineage(object): def __init__(self, tool_id, **kwds): self.tool_id = tool_id - self._tool_versions = set() - - @property - def tool_versions(self): - return sorted(self._tool_versions, key=packaging.version.parse) + self.tool_versions = SortedSet(key=packaging.version.parse) @property def tool_ids(self): @@ -64,7 +61,7 @@ class ToolLineage(object): def register_version(self, tool_version): assert tool_version is not None - self._tool_versions.add(str(tool_version)) + self.tool_versions.add(str(tool_version)) def get_versions(self): """ From 788948b9546157e9f18231e635bc0be4bdf1fad2 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Wed, 15 Apr 2020 20:47:05 +0200 Subject: [PATCH 23/33] Move tool source cache to tools.cache --- lib/galaxy/tool_util/parser/xml.py | 3 ++ lib/galaxy/tools/__init__.py | 78 ++---------------------------- lib/galaxy/tools/cache.py | 76 ++++++++++++++++++++++++++++- 3 files changed, 83 insertions(+), 74 deletions(-) diff --git a/lib/galaxy/tool_util/parser/xml.py b/lib/galaxy/tool_util/parser/xml.py index 7f9a4af5c1a..15d6da7f48a 100644 --- a/lib/galaxy/tool_util/parser/xml.py +++ b/lib/galaxy/tool_util/parser/xml.py @@ -50,6 +50,9 @@ class XmlToolSource(ToolSource): self._macro_paths = macro_paths or [] self.legacy_defaults = self.parse_profile() == "16.01" + def to_string(self): + return xml_to_string(self.root) + def parse_version(self): return self.root.get("version", None) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 9c73f26caf0..25aa8c775f7 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -20,15 +20,6 @@ from xml.etree import ElementTree import packaging.version import webob.exc -from dogpile.cache import make_region -from dogpile.cache.api import ( - CachedValue, - NO_VALUE, -) -from dogpile.cache.backends.file import AbstractFileLock -from dogpile.cache.proxy import ProxyBackend -from dogpile.util import ReadWriteMutex -from lxml import etree from mako.template import Template from six import itervalues, string_types from six.moves.urllib.parse import unquote_plus @@ -66,6 +57,11 @@ from galaxy.tools.actions import DefaultToolAction from galaxy.tools.actions.data_manager import DataManagerToolAction from galaxy.tools.actions.data_source import DataSourceToolAction from galaxy.tools.actions.model_operations import ModelOperationToolAction +from galaxy.tools.cache import ( + JSONBackend, + MutexLock, + region, +) from galaxy.tools.parameters import ( check_param, params_from_strings, @@ -124,70 +120,6 @@ from .execute import ( ) -class JSONBackend(ProxyBackend): - - def set(self, key, value): - with self.proxied._dbm_file(True) as dbm: - dbm[key] = json.dumps({'metadata': value.metadata, 'payload': self.value_encode(value), 'macro_paths': value.payload.macro_paths()}) - - def get(self, key): - with self.proxied._dbm_file(False) as dbm: - if hasattr(dbm, "get"): - value = dbm.get(key, NO_VALUE) - else: - # gdbm objects lack a .get method - try: - value = dbm[key] - except KeyError: - value = NO_VALUE - if value is not NO_VALUE: - value = self.value_decode(value) - return value - - def value_decode(self, v): - if not v or v is NO_VALUE: - return NO_VALUE - # v is returned as bytestring, so we need to `unicodify` on python < 3.6 before we can use json.loads - v = json.loads(unicodify(v)) - payload = get_tool_source(xml_tree=etree.ElementTree(etree.fromstring(v['payload'].encode('utf-8'))), macro_paths=v['macro_paths']) - return CachedValue(metadata=v['metadata'], payload=payload) - - def value_encode(self, v): - payload = ElementTree.tostring(v.payload.root, encoding="utf-8", method='xml').decode('utf-8') - return payload - - -class MutexLock(AbstractFileLock): - def __init__(self, filename): - self.mutex = ReadWriteMutex() - - def acquire_read_lock(self, wait): - # No need for read lock. It is supposed to prevent the "dogpile" effect - # where multiple functions each create the cached resource, but I don't - # think we care. - return True - - def acquire_write_lock(self, wait): - ret = self.mutex.acquire_write_lock(wait) - return wait or ret - - def release_read_lock(self): - return True - - def release_write_lock(self): - return self.mutex.release_write_lock() - - -def my_key_generator(namespace, fn, **kw): - - def generate_key(*arg): - return "_".join(str(s) for s in arg if isinstance(s, str)) - - return generate_key - - -region = make_region(function_key_generator=my_key_generator) - log = logging.getLogger(__name__) REQUIRES_JS_RUNTIME_MESSAGE = ("The tool [%s] requires a nodejs runtime to execute " diff --git a/lib/galaxy/tools/cache.py b/lib/galaxy/tools/cache.py index 59ee6b1d1f5..0dc62a21fa3 100644 --- a/lib/galaxy/tools/cache.py +++ b/lib/galaxy/tools/cache.py @@ -1,20 +1,94 @@ +import json import logging import os from collections import defaultdict from threading import Lock +from dogpile.cache import make_region +from dogpile.cache.api import ( + CachedValue, + NO_VALUE, +) +from dogpile.cache.backends.file import AbstractFileLock +from dogpile.cache.proxy import ProxyBackend +from dogpile.util import ReadWriteMutex +from lxml import etree from sqlalchemy.orm import ( defer, joinedload, ) -from galaxy.tools import region +from galaxy.tool_util.parser import get_tool_source from galaxy.util import unicodify from galaxy.util.hash_util import md5_hash_file log = logging.getLogger(__name__) +class JSONBackend(ProxyBackend): + + def set(self, key, value): + with self.proxied._dbm_file(True) as dbm: + dbm[key] = json.dumps({'metadata': value.metadata, 'payload': self.value_encode(value), 'macro_paths': value.payload.macro_paths()}) + + def get(self, key): + with self.proxied._dbm_file(False) as dbm: + if hasattr(dbm, "get"): + value = dbm.get(key, NO_VALUE) + else: + # gdbm objects lack a .get method + try: + value = dbm[key] + except KeyError: + value = NO_VALUE + if value is not NO_VALUE: + value = self.value_decode(value) + return value + + def value_decode(self, v): + if not v or v is NO_VALUE: + return NO_VALUE + # v is returned as bytestring, so we need to `unicodify` on python < 3.6 before we can use json.loads + v = json.loads(unicodify(v)) + payload = get_tool_source(xml_tree=etree.ElementTree(etree.fromstring(v['payload'].encode('utf-8'))), macro_paths=v['macro_paths']) + return CachedValue(metadata=v['metadata'], payload=payload) + + def value_encode(self, v): + return unicodify(v.payload.to_string()) + + +class MutexLock(AbstractFileLock): + def __init__(self, filename): + self.mutex = ReadWriteMutex() + + def acquire_read_lock(self, wait): + # No need for read lock. It is supposed to prevent the "dogpile" effect + # where multiple functions each create the cached resource, but I don't + # think we care. + return True + + def acquire_write_lock(self, wait): + ret = self.mutex.acquire_write_lock(wait) + return wait or ret + + def release_read_lock(self): + return True + + def release_write_lock(self): + return self.mutex.release_write_lock() + + +def my_key_generator(namespace, fn, **kw): + + def generate_key(*arg): + return "_".join(str(s) for s in arg if isinstance(s, str)) + + return generate_key + + +region = make_region(function_key_generator=my_key_generator) + + class ToolCache(object): """ Cache tool definitions to allow quickly reloading the whole From 439e2143855fa559abb4cd12e6de504e3b0a3e22 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 16 Apr 2020 12:15:01 +0200 Subject: [PATCH 24/33] Iterate over copy of tools dictionary, avoids issues if dictionary size changes --- lib/galaxy/tools/toolbox/base.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/galaxy/tools/toolbox/base.py b/lib/galaxy/tools/toolbox/base.py index a70f75c40a5..0dcbae0313b 100644 --- a/lib/galaxy/tools/toolbox/base.py +++ b/lib/galaxy/tools/toolbox/base.py @@ -11,7 +11,6 @@ from errno import ENOENT from xml.etree.ElementTree import ParseError from markupsafe import escape -from six import iteritems from six.moves.urllib.parse import urlparse from galaxy.exceptions import ( @@ -597,7 +596,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): return [] def tools(self): - return iteritems(self._tools_by_id) + return self._tools_by_id.copy().items() def dynamic_confs(self, include_migrated_tool_conf=False): confs = [] From 8ed00d922aba70c589e935ac559e269e64c492d3 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 16 Apr 2020 12:49:14 +0200 Subject: [PATCH 25/33] Make delay_tool_initialiation a config option Delaying the tool parameter parsing until after the fork means higher memory consumption in each forked process as all those parameters are kept in memory. For main's toolbox that amounts to about 260 MB of memory. I guess I'll make this configurable, on on-demand instances you probably rarely ever finalize all the tools (you can easily do that though, by callings /api/tools/tests_summary for instance), so that would actually save some memory. If you never fork (for instance you run handlers with ./scripts/galaxy-main) it would also not lead to increased memory usage. --- doc/source/admin/galaxy_options.rst | 12 ++++++++++++ lib/galaxy/config/sample/galaxy.yml.sample | 5 +++++ lib/galaxy/tools/__init__.py | 6 ++++-- lib/galaxy/webapps/galaxy/config_schema.yml | 9 +++++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst index 514c3036b68..a8b0988e9c7 100644 --- a/doc/source/admin/galaxy_options.rst +++ b/doc/source/admin/galaxy_options.rst @@ -1013,6 +1013,18 @@ :Type: str +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +``delay_tool_initialization`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +:Description: + Set this to true to delay parsing of tool inputs and outputs until + they are needed. This results in faster startup times but uses + more memory when using forked Galaxy processes. +:Default: ``false`` +:Type: bool + + ~~~~~~~~~~~~~~~~~~~~~~~ ``citation_cache_type`` ~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample index 8950280601e..5ebadf53d76 100644 --- a/lib/galaxy/config/sample/galaxy.yml.sample +++ b/lib/galaxy/config/sample/galaxy.yml.sample @@ -592,6 +592,11 @@ galaxy: # at this path. #tool_cache_data_dir: tool_cache + # Set this to true to delay parsing of tool inputs and outputs until + # they are needed. This results in faster startup times but uses more + # memory when using forked Galaxy processes. + #delay_tool_initialization: false + # Citation related caching. Tool citations information maybe fetched # from external sources such as https://doi.org/ by Galaxy - the # following parameters can be used to control the caching used to diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 25aa8c775f7..bf5c34a7023 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -301,9 +301,11 @@ class ToolBox(BaseGalaxyToolBox): return self._tools_by_id def create_tool(self, config_file, **kwds): - tool_source = self.get_expanded_tool_source(config_file) - return self._create_tool_from_source(tool_source, config_file=config_file, **kwds) + tool = self._create_tool_from_source(tool_source, config_file=config_file, **kwds) + if not self.app.config.delay_tool_initialization: + tool.assert_finalized() + return tool @region.cache_on_arguments() def get_expanded_tool_source(self, config_file): diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml index cbdcfd0cda5..32a34949bab 100644 --- a/lib/galaxy/webapps/galaxy/config_schema.yml +++ b/lib/galaxy/webapps/galaxy/config_schema.yml @@ -757,6 +757,15 @@ mapping: desc: | Tool related caching. Full expanded tools and metadata wll be stored at this path. + delay_tool_initialization: + type: bool + default: false + required: false + desc: | + Set this to true to delay parsing of tool inputs and outputs until they are needed. + This results in faster startup times but uses more memory when using forked Galaxy + processes. + citation_cache_type: type: str default: file From d1157787ff5feadd2bfb2878d0a8fc14484dda99 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 16 Apr 2020 16:44:23 +0200 Subject: [PATCH 26/33] Remove tool from toolbox if finalizing tool parameters failed --- lib/galaxy/tool_shed/tools/tool_validator.py | 2 +- lib/galaxy/tools/__init__.py | 19 ++++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/galaxy/tool_shed/tools/tool_validator.py b/lib/galaxy/tool_shed/tools/tool_validator.py index 495bb277d0f..9dd6ff5353d 100644 --- a/lib/galaxy/tool_shed/tools/tool_validator.py +++ b/lib/galaxy/tool_shed/tools/tool_validator.py @@ -78,7 +78,7 @@ class ToolValidator(object): ) try: tool = create_tool_from_source(config_file=full_path, app=self.app, tool_source=tool_source, repository_id=repository_id, allow_code_files=False) - tool.assert_finalized() + tool.assert_finalized(raise_if_invalid=True) valid = True error_message = None except KeyError as e: diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index bf5c34a7023..c500c211b1a 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -304,7 +304,7 @@ class ToolBox(BaseGalaxyToolBox): tool_source = self.get_expanded_tool_source(config_file) tool = self._create_tool_from_source(tool_source, config_file=config_file, **kwds) if not self.app.config.delay_tool_initialization: - tool.assert_finalized() + tool.assert_finalized(raise_if_invalid=True) return tool @region.cache_on_arguments() @@ -552,11 +552,20 @@ class Tool(Dictifiable): return getattr(self, name) raise AttributeError(name) - def assert_finalized(self): + def assert_finalized(self, raise_if_invalid=False): if self.finalized is False: - self.parse_inputs(self.tool_source) - self.parse_outputs(self.tool_source) - self.finalized = True + try: + self.parse_inputs(self.tool_source) + self.parse_outputs(self.tool_source) + self.finalized = True + except Exception: + toolbox = getattr(self.app, 'toolbox', None) + if toolbox: + toolbox.remove_tool_by_id(self.id) + if raise_if_invalid: + raise + else: + log.warning("An error occured while parsing the tool wrapper xml, the tool is not functional", exc_info=True) @property def history_manager(self): From 14cb1fab220efbe375c34034b478c238e76bb40d Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Thu, 16 Apr 2020 18:15:44 +0200 Subject: [PATCH 27/33] Fix unit tests --- test/unit/unittest_utils/galaxy_mock.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/unit/unittest_utils/galaxy_mock.py b/test/unit/unittest_utils/galaxy_mock.py index b579fcd09aa..83274384b9c 100644 --- a/test/unit/unittest_utils/galaxy_mock.py +++ b/test/unit/unittest_utils/galaxy_mock.py @@ -171,6 +171,7 @@ class MockAppConfig(Bunch): # set by MockDir self.root = root self.tool_cache_data_dir = os.path.join(root, 'tool_cache') + self.delay_tool_initialization = True self.config_file = None From c21a7c3be877d8afea319f70c8e3aaa016bdd23c Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 Apr 2020 11:12:17 +0200 Subject: [PATCH 28/33] Remove unused interal argument to toolbox.load_item --- lib/galaxy/tools/toolbox/base.py | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/lib/galaxy/tools/toolbox/base.py b/lib/galaxy/tools/toolbox/base.py index 0dcbae0313b..c5bb9092c92 100644 --- a/lib/galaxy/tools/toolbox/base.py +++ b/lib/galaxy/tools/toolbox/base.py @@ -211,7 +211,6 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): load_panel_dict=load_panel_dict, guid=item.get('guid'), index=index, - internal=True ) if parsing_shed_tool_conf: @@ -239,25 +238,20 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): self._tools_by_uuid[dynamic_tool.uuid] = tool return tool - def load_item(self, item, tool_path, panel_dict=None, integrated_panel_dict=None, load_panel_dict=True, guid=None, index=None, internal=False): + def load_item(self, item, tool_path, panel_dict=None, integrated_panel_dict=None, load_panel_dict=True, guid=None, index=None): with self.app._toolbox_lock: item = ensure_tool_conf_item(item) item_type = item.type - if item_type not in ['tool', 'section'] and not internal: - # External calls from tool shed code cannot load labels or tool - # directories. - return - if panel_dict is None: panel_dict = self._tool_panel if integrated_panel_dict is None: integrated_panel_dict = self._integrated_tool_panel if item_type == 'tool': - self._load_tool_tag_set(item, panel_dict=panel_dict, integrated_panel_dict=integrated_panel_dict, tool_path=tool_path, load_panel_dict=load_panel_dict, guid=guid, index=index, internal=internal) + self._load_tool_tag_set(item, panel_dict=panel_dict, integrated_panel_dict=integrated_panel_dict, tool_path=tool_path, load_panel_dict=load_panel_dict, guid=guid, index=index) elif item_type == 'workflow': self._load_workflow_tag_set(item, panel_dict=panel_dict, integrated_panel_dict=integrated_panel_dict, load_panel_dict=load_panel_dict, index=index) elif item_type == 'section': - self._load_section_tag_set(item, tool_path=tool_path, load_panel_dict=load_panel_dict, index=index, internal=internal) + self._load_section_tag_set(item, tool_path=tool_path, load_panel_dict=load_panel_dict, index=index) elif item_type == 'label': self._load_label_tag_set(item, panel_dict=panel_dict, integrated_panel_dict=integrated_panel_dict, load_panel_dict=load_panel_dict, index=index) elif item_type == 'tool_dir': @@ -632,7 +626,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): def _path_template_kwds(self): return {} - def _load_tool_tag_set(self, item, panel_dict, integrated_panel_dict, tool_path, load_panel_dict, guid=None, index=None, internal=False): + def _load_tool_tag_set(self, item, panel_dict, integrated_panel_dict, tool_path, load_panel_dict, guid=None, index=None): try: path_template = item.get("file") template_kwds = self._path_template_kwds() @@ -766,7 +760,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): panel_dict[key] = label integrated_panel_dict.update_or_append(index, key, label) - def _load_section_tag_set(self, item, tool_path, load_panel_dict, index=None, internal=False): + def _load_section_tag_set(self, item, tool_path, load_panel_dict, index=None): key = item.get("id") if key in self._tool_panel: section = self._tool_panel[key] @@ -789,7 +783,6 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): load_panel_dict=load_panel_dict, guid=sub_item.get('guid'), index=sub_index, - internal=internal, ) # Ensure each tool's section is stored From b71d4d6997cc849bd1e8c26b1b97b528271d1b26 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 Apr 2020 12:45:29 +0200 Subject: [PATCH 29/33] Implement tool source caching per shed_tool_conf That should make it easy to distribute the cache on cvmfs. --- lib/galaxy/tools/__init__.py | 34 ++++++++++++------------- lib/galaxy/tools/cache.py | 41 +++++++++++++++++++----------- lib/galaxy/tools/toolbox/base.py | 31 ++++++++++++---------- lib/galaxy/tools/toolbox/parser.py | 6 +++++ 4 files changed, 65 insertions(+), 47 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index c500c211b1a..d08a3dcb13b 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -57,11 +57,7 @@ from galaxy.tools.actions import DefaultToolAction from galaxy.tools.actions.data_manager import DataManagerToolAction from galaxy.tools.actions.data_source import DataSourceToolAction from galaxy.tools.actions.model_operations import ModelOperationToolAction -from galaxy.tools.cache import ( - JSONBackend, - MutexLock, - region, -) +from galaxy.tools.cache import create_cache_region from galaxy.tools.parameters import ( check_param, params_from_strings, @@ -250,18 +246,9 @@ class ToolBox(BaseGalaxyToolBox): def __init__(self, config_filenames, tool_root_dir, app, save_integrated_tool_panel=True): self._reload_count = 0 self.tool_location_fetcher = ToolLocationFetcher() + self.cache_regions = {} if not os.path.exists(app.config.tool_cache_data_dir): os.makedirs(app.config.tool_cache_data_dir) - region.configure( - 'dogpile.cache.dbm', - arguments={ - "filename": os.path.join(app.config.tool_cache_data_dir, "cache.dbm"), - "lock_factory": MutexLock, - }, - expiration_time=-1, - wrap=[JSONBackend], - replace_existing_backend=True, - ) # This is here to deal with the old default value, which doesn't make # sense in an "installed Galaxy" world. # FIXME: ./ @@ -300,14 +287,19 @@ class ToolBox(BaseGalaxyToolBox): # Deprecated method, TODO - eliminate calls to this in test/. return self._tools_by_id - def create_tool(self, config_file, **kwds): - tool_source = self.get_expanded_tool_source(config_file) + def get_cache_region(self, tool_cache_data_dir): + if tool_cache_data_dir not in self.cache_regions: + self.cache_regions[tool_cache_data_dir] = create_cache_region(tool_cache_data_dir) + return self.cache_regions[tool_cache_data_dir] + + def create_tool(self, config_file, tool_cache_data_dir=None, **kwds): + cache = self.get_cache_region(tool_cache_data_dir or self.app.config.tool_cache_data_dir) + tool_source = cache.get_or_create(config_file, creator=self.get_expanded_tool_source, expiration_time=-1, creator_args=((config_file,), {})) tool = self._create_tool_from_source(tool_source, config_file=config_file, **kwds) if not self.app.config.delay_tool_initialization: tool.assert_finalized(raise_if_invalid=True) return tool - @region.cache_on_arguments() def get_expanded_tool_source(self, config_file): try: return get_tool_source( @@ -567,6 +559,12 @@ class Tool(Dictifiable): else: log.warning("An error occured while parsing the tool wrapper xml, the tool is not functional", exc_info=True) + def remove_from_cache(self): + source_path = self.tool_source._source_path + if source_path: + for region in self.app.toolbox.cache_regions.values(): + region.delete(source_path) + @property def history_manager(self): return self.app.history_manager diff --git a/lib/galaxy/tools/cache.py b/lib/galaxy/tools/cache.py index 0dc62a21fa3..871173a90fe 100644 --- a/lib/galaxy/tools/cache.py +++ b/lib/galaxy/tools/cache.py @@ -42,15 +42,19 @@ class JSONBackend(ProxyBackend): except KeyError: value = NO_VALUE if value is not NO_VALUE: - value = self.value_decode(value) + value = self.value_decode(key, value) return value - def value_decode(self, v): + def value_decode(self, k, v): if not v or v is NO_VALUE: return NO_VALUE # v is returned as bytestring, so we need to `unicodify` on python < 3.6 before we can use json.loads v = json.loads(unicodify(v)) - payload = get_tool_source(xml_tree=etree.ElementTree(etree.fromstring(v['payload'].encode('utf-8'))), macro_paths=v['macro_paths']) + payload = get_tool_source( + config_file=k, + xml_tree=etree.ElementTree(etree.fromstring(v['payload'].encode('utf-8'))), + macro_paths=v['macro_paths'] + ) return CachedValue(metadata=v['metadata'], payload=payload) def value_encode(self, v): @@ -78,15 +82,21 @@ class MutexLock(AbstractFileLock): return self.mutex.release_write_lock() -def my_key_generator(namespace, fn, **kw): - - def generate_key(*arg): - return "_".join(str(s) for s in arg if isinstance(s, str)) - - return generate_key - - -region = make_region(function_key_generator=my_key_generator) +def create_cache_region(tool_cache_data_dir): + if not os.path.exists(tool_cache_data_dir): + os.makedirs(tool_cache_data_dir) + region = make_region() + region.configure( + 'dogpile.cache.dbm', + arguments={ + "filename": os.path.join(tool_cache_data_dir, "cache.dbm"), + "lock_factory": MutexLock, + }, + expiration_time=-1, + wrap=[JSONBackend], + replace_existing_backend=True, + ) + return region class ToolCache(object): @@ -121,15 +131,16 @@ class ToolCache(object): removed_tool_ids = [] try: with self._lock: - paths_to_cleanup = {path: tool.all_ids for path, tool in self._tools_by_path.items() if self._should_cleanup(path)} - for config_filename, tool_ids in paths_to_cleanup.items(): - region.delete(config_filename) + paths_to_cleanup = {(path, tool) for path, tool in self._tools_by_path.items() if self._should_cleanup(path)} + for config_filename, tool in paths_to_cleanup: + tool.remove_from_cache() del self._hash_by_tool_paths[config_filename] if os.path.exists(config_filename): # This tool has probably been broken while editing on disk # We record it here, so that we can recover it self._removed_tools_by_path[config_filename] = self._tools_by_path[config_filename] del self._tools_by_path[config_filename] + tool_ids = tool.all_ids for tool_id in tool_ids: if tool_id in self._tool_paths_by_id: del self._tool_paths_by_id[tool_id] diff --git a/lib/galaxy/tools/toolbox/base.py b/lib/galaxy/tools/toolbox/base.py index c5bb9092c92..a21482d2ba4 100644 --- a/lib/galaxy/tools/toolbox/base.py +++ b/lib/galaxy/tools/toolbox/base.py @@ -191,6 +191,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): return raise tool_path = tool_conf_source.parse_tool_path() + tool_cache_data_dir = tool_conf_source.parse_tool_cache_data_dir() parsing_shed_tool_conf = tool_conf_source.is_shed_tool_conf() if parsing_shed_tool_conf: # Keep an in-memory list of xml elements to enable persistence of the changing tool config. @@ -208,6 +209,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): self.load_item( item, tool_path=tool_path, + tool_cache_data_dir=tool_cache_data_dir, load_panel_dict=load_panel_dict, guid=item.get('guid'), index=index, @@ -238,7 +240,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): self._tools_by_uuid[dynamic_tool.uuid] = tool return tool - def load_item(self, item, tool_path, panel_dict=None, integrated_panel_dict=None, load_panel_dict=True, guid=None, index=None): + def load_item(self, item, tool_path, panel_dict=None, integrated_panel_dict=None, load_panel_dict=True, guid=None, index=None, tool_cache_data_dir=None): with self.app._toolbox_lock: item = ensure_tool_conf_item(item) item_type = item.type @@ -247,15 +249,15 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): if integrated_panel_dict is None: integrated_panel_dict = self._integrated_tool_panel if item_type == 'tool': - self._load_tool_tag_set(item, panel_dict=panel_dict, integrated_panel_dict=integrated_panel_dict, tool_path=tool_path, load_panel_dict=load_panel_dict, guid=guid, index=index) + self._load_tool_tag_set(item, panel_dict=panel_dict, integrated_panel_dict=integrated_panel_dict, tool_path=tool_path, load_panel_dict=load_panel_dict, guid=guid, index=index, tool_cache_data_dir=tool_cache_data_dir) elif item_type == 'workflow': self._load_workflow_tag_set(item, panel_dict=panel_dict, integrated_panel_dict=integrated_panel_dict, load_panel_dict=load_panel_dict, index=index) elif item_type == 'section': - self._load_section_tag_set(item, tool_path=tool_path, load_panel_dict=load_panel_dict, index=index) + self._load_section_tag_set(item, tool_path=tool_path, load_panel_dict=load_panel_dict, index=index, tool_cache_data_dir=tool_cache_data_dir) elif item_type == 'label': self._load_label_tag_set(item, panel_dict=panel_dict, integrated_panel_dict=integrated_panel_dict, load_panel_dict=load_panel_dict, index=index) elif item_type == 'tool_dir': - self._load_tooldir_tag_set(item, panel_dict, tool_path, integrated_panel_dict, load_panel_dict=load_panel_dict) + self._load_tooldir_tag_set(item, panel_dict, tool_path, integrated_panel_dict, load_panel_dict=load_panel_dict, tool_cache_data_dir=tool_cache_data_dir) def get_shed_config_dict_by_filename(self, filename): filename = os.path.abspath(filename) @@ -626,7 +628,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): def _path_template_kwds(self): return {} - def _load_tool_tag_set(self, item, panel_dict, integrated_panel_dict, tool_path, load_panel_dict, guid=None, index=None): + def _load_tool_tag_set(self, item, panel_dict, integrated_panel_dict, tool_path, load_panel_dict, guid=None, index=None, tool_cache_data_dir=None): try: path_template = item.get("file") template_kwds = self._path_template_kwds() @@ -652,9 +654,9 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): # The shed tool is in the install database # Only load tools if the repository is not deactivated or uninstalled. can_load_into_panel_dict = not tool_shed_repository.deleted - tool = self.load_tool(concrete_path, guid=guid, tool_shed_repository=tool_shed_repository, use_cached=False) + tool = self.load_tool(concrete_path, guid=guid, tool_shed_repository=tool_shed_repository, use_cached=False, tool_cache_data_dir=tool_cache_data_dir) if not tool: # tool was not in cache and is not a tool shed tool. - tool = self.load_tool(concrete_path, use_cached=False) + tool = self.load_tool(concrete_path, use_cached=False, tool_cache_data_dir=tool_cache_data_dir) if string_as_bool(item.get('hidden', False)): tool.hidden = True key = 'tool_%s' % str(tool.id) @@ -760,7 +762,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): panel_dict[key] = label integrated_panel_dict.update_or_append(index, key, label) - def _load_section_tag_set(self, item, tool_path, load_panel_dict, index=None): + def _load_section_tag_set(self, item, tool_path, load_panel_dict, index=None, tool_cache_data_dir=None): key = item.get("id") if key in self._tool_panel: section = self._tool_panel[key] @@ -783,6 +785,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): load_panel_dict=load_panel_dict, guid=sub_item.get('guid'), index=sub_index, + tool_cache_data_dir=tool_cache_data_dir, ) # Ensure each tool's section is stored @@ -797,16 +800,16 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): # Always load sections into the integrated_tool_panel. self._integrated_tool_panel.update_or_append(index, key, integrated_section) - def _load_tooldir_tag_set(self, item, elems, tool_path, integrated_elems, load_panel_dict): + def _load_tooldir_tag_set(self, item, elems, tool_path, integrated_elems, load_panel_dict, tool_cache_data_dir=None): directory = os.path.join(tool_path, item.get("dir")) recursive = string_as_bool(item.get("recursive", True)) - self.__watch_directory(directory, elems, integrated_elems, load_panel_dict, recursive, force_watch=True) + self.__watch_directory(directory, elems, integrated_elems, load_panel_dict, recursive, force_watch=True, tool_cache_data_dir=tool_cache_data_dir) - def __watch_directory(self, directory, elems, integrated_elems, load_panel_dict, recursive, force_watch=False): + def __watch_directory(self, directory, elems, integrated_elems, load_panel_dict, recursive, force_watch=False, tool_cache_data_dir=None): def quick_load(tool_file, async_load=True): try: - tool = self.load_tool(tool_file) + tool = self.load_tool(tool_file, tool_cache_data_dir) self.__add_tool(tool, load_panel_dict, elems) # Always load the tool into the integrated_panel_dict, or it will not be included in the integrated_tool_panel.xml file. key = 'tool_%s' % str(tool.id) @@ -837,7 +840,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): if (tool_loaded or force_watch) and self._tool_watcher: self._tool_watcher.watch_directory(directory, quick_load) - def load_tool(self, config_file, guid=None, tool_shed_repository=None, use_cached=False, **kwds): + def load_tool(self, config_file, guid=None, tool_shed_repository=None, use_cached=False, tool_cache_data_dir=None, **kwds): """Load a single tool from the file named by `config_file` and return an instance of `Tool`.""" # Parse XML configuration file and get the root element tool = None @@ -845,7 +848,7 @@ class AbstractToolBox(Dictifiable, ManagesIntegratedToolPanelMixin): tool = self.load_tool_from_cache(config_file) if not tool or guid and guid != tool.guid: try: - tool = self.create_tool(config_file=config_file, tool_shed_repository=tool_shed_repository, guid=guid, **kwds) + tool = self.create_tool(config_file=config_file, tool_shed_repository=tool_shed_repository, guid=guid, tool_cache_data_dir=tool_cache_data_dir, **kwds) except Exception: # If the tool is broken but still exists we can load it from the cache tool = self.load_tool_from_cache(config_file, recover_tool=True) diff --git a/lib/galaxy/tools/toolbox/parser.py b/lib/galaxy/tools/toolbox/parser.py index db1b582404c..95ae8b55864 100644 --- a/lib/galaxy/tools/toolbox/parser.py +++ b/lib/galaxy/tools/toolbox/parser.py @@ -43,6 +43,9 @@ class XmlToolConfSource(ToolConfSource): def parse_tool_path(self): return self.root.get('tool_path') + def parse_tool_cache_data_dir(self): + return self.root.get('tool_cache_data_dir') + def parse_items(self): return [ensure_tool_conf_item(_) for _ in self.root] @@ -65,6 +68,9 @@ class YamlToolConfSource(ToolConfSource): def parse_tool_path(self): return self.as_dict.get('tool_path') + def parse_tool_cache_data_dir(self): + return self.as_dict.get('tool_cache_data_dir') + def parse_items(self): return [ToolConfItem.from_dict(_) for _ in self.as_dict.get('items')] From fd17a19f5165e19204fd94aee7ad09be9a862846 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 17 Apr 2020 14:41:54 +0200 Subject: [PATCH 30/33] Mention that tool_cache_data_dir can be set in tool_conf.xml files --- doc/source/admin/galaxy_options.rst | 6 ++++-- lib/galaxy/config/sample/galaxy.yml.sample | 6 ++++-- lib/galaxy/webapps/galaxy/config_schema.yml | 4 +++- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/doc/source/admin/galaxy_options.rst b/doc/source/admin/galaxy_options.rst index a8b0988e9c7..4f64dfb17a6 100644 --- a/doc/source/admin/galaxy_options.rst +++ b/doc/source/admin/galaxy_options.rst @@ -1007,8 +1007,10 @@ ~~~~~~~~~~~~~~~~~~~~~~~ :Description: - Tool related caching. Full expanded tools and metadata wll be - stroed at this path. + Tool related caching. Fully expanded tools and metadata will be + stored at this path. Per tool_conf cache locations can be + configured in (shed_)tool_conf.xml files using the + tool_cache_data_dir attribute. :Default: ``tool_cache`` :Type: str diff --git a/lib/galaxy/config/sample/galaxy.yml.sample b/lib/galaxy/config/sample/galaxy.yml.sample index 5ebadf53d76..186ba66fe93 100644 --- a/lib/galaxy/config/sample/galaxy.yml.sample +++ b/lib/galaxy/config/sample/galaxy.yml.sample @@ -588,8 +588,10 @@ galaxy: # generated commands run in sh. #default_job_shell: /bin/bash - # Tool related caching. Full expanded tools and metadata wll be stroed - # at this path. + # Tool related caching. Fully expanded tools and metadata will be + # stored at this path. Per tool_conf cache locations can be configured + # in (shed_)tool_conf.xml files using the tool_cache_data_dir + # attribute. #tool_cache_data_dir: tool_cache # Set this to true to delay parsing of tool inputs and outputs until diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml index 32a34949bab..6a01b393314 100644 --- a/lib/galaxy/webapps/galaxy/config_schema.yml +++ b/lib/galaxy/webapps/galaxy/config_schema.yml @@ -755,7 +755,9 @@ mapping: path_resolves_to: data_dir required: false desc: | - Tool related caching. Full expanded tools and metadata wll be stored at this path. + Tool related caching. Fully expanded tools and metadata will be stored at this path. + Per tool_conf cache locations can be configured in (shed_)tool_conf.xml files using + the tool_cache_data_dir attribute. delay_tool_initialization: type: bool From 19c4ff42823638d51792a98486cb28e9b92a6923 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 18 Apr 2020 18:07:23 +0200 Subject: [PATCH 31/33] Store tool search index This means we also need to remove indexed documents that don't exist anymore. --- lib/galaxy/config/__init__.py | 2 +- lib/galaxy/tools/search/__init__.py | 55 ++++++++++++--------- lib/galaxy/webapps/galaxy/config_schema.yml | 8 +++ lib/tool_shed/util/shed_index.py | 19 ++----- 4 files changed, 44 insertions(+), 40 deletions(-) diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py index e0412f5b188..1beb038a4aa 100644 --- a/lib/galaxy/config/__init__.py +++ b/lib/galaxy/config/__init__.py @@ -1031,7 +1031,7 @@ class ConfiguresGalaxyMixin(object): self.container_finder = containers.ContainerFinder(app_info, mulled_resolution_cache=mulled_resolution_cache) self._set_enabled_container_types() index_help = getattr(self.config, "index_tool_help", True) - self.toolbox_search = galaxy.tools.search.ToolBoxSearch(self.toolbox, index_help) + self.toolbox_search = galaxy.tools.search.ToolBoxSearch(self.toolbox, index_dir=self.config.tool_search_index_dir, index_help=index_help) def reindex_tool_search(self): # Call this when tools are added or removed. diff --git a/lib/galaxy/tools/search/__init__.py b/lib/galaxy/tools/search/__init__.py index 85afae25702..71482fa8302 100644 --- a/lib/galaxy/tools/search/__init__.py +++ b/lib/galaxy/tools/search/__init__.py @@ -5,24 +5,24 @@ or searching related parts it is deeply recommended to read through the library docs at https://whoosh.readthedocs.io. """ import logging +import os import re -import tempfile -from whoosh import analysis +from whoosh import ( + analysis, + index, +) from whoosh.analysis import StandardAnalyzer from whoosh.fields import ( + ID, KEYWORD, Schema, - STORED, TEXT ) -from whoosh.filedb.filestore import ( - FileStorage, - RamStorage -) from whoosh.qparser import MultifieldParser from whoosh.qparser import OrGroup from whoosh.scoring import BM25F +from whoosh.writing import AsyncWriter from galaxy.util import ExecutionTimer from galaxy.web.framework.helpers import to_unicode @@ -30,14 +30,27 @@ from galaxy.web.framework.helpers import to_unicode log = logging.getLogger(__name__) +def get_or_create_index(index_dir, schema): + if not os.path.exists(index_dir): + os.makedirs(index_dir) + if index.exists_in(index_dir): + idx = index.open_dir(index_dir) + try: + assert idx.schema == schema + return idx + except AssertionError: + log.warning("Index at '%s' uses outdated schema, creating new index", index_dir) + return index.create_in(index_dir, schema=schema) + + class ToolBoxSearch(object): """ Support searching tools in a toolbox. This implementation uses the Whoosh search library. """ - def __init__(self, toolbox, index_help=True): - self.schema = Schema(id=STORED, + def __init__(self, toolbox, index_dir=None, index_help=True): + self.schema = Schema(id=ID(stored=True), stub=KEYWORD, name=TEXT(analyzer=analysis.SimpleAnalyzer()), description=TEXT, @@ -45,8 +58,9 @@ class ToolBoxSearch(object): help=TEXT, labels=KEYWORD) self.rex = analysis.RegexTokenizer() + self.index_dir = index_dir self.toolbox = toolbox - self.storage, self.index = self._index_setup() + self.index = self._index_setup() # We keep track of how many times the tool index has been rebuilt. # We start at -1, so that after the first index the count is at 0, # which is the same as the toolbox reload count. This way we can skip @@ -54,11 +68,7 @@ class ToolBoxSearch(object): self.index_count = -1 def _index_setup(self): - RamStorage.temp_storage = _temp_storage - # Works around https://bitbucket.org/mchaput/whoosh/issues/391/race-conditions-with-temp-storage - storage = RamStorage() - index = storage.create_index(self.schema) - return storage, index + return get_or_create_index(index_dir=self.index_dir, schema=self.schema) def build_index(self, tool_cache, index_help=True): """ @@ -68,10 +78,13 @@ class ToolBoxSearch(object): log.debug('Starting to build toolbox index.') self.index_count += 1 execution_timer = ExecutionTimer() - writer = self.index.writer() - for tool_id in tool_cache._removed_tool_ids: + with self.index.searcher() as searcher: + indexed_tool_ids = {f.get('id') for f in searcher.all_stored_fields()} + tool_ids_to_remove = (indexed_tool_ids - set(tool_cache._tool_paths_by_id.keys())).union(tool_cache._removed_tool_ids) + writer = AsyncWriter(self.index) + for tool_id in tool_ids_to_remove: writer.delete_by_term('id', tool_id) - for tool_id in tool_cache._new_tool_ids: + for tool_id in tool_cache._new_tool_ids - indexed_tool_ids: tool = tool_cache.get_tool_by_id(tool_id) if tool and tool.is_latest_version: add_doc_kwds = self._create_doc(tool_id=tool_id, tool=tool, index_help=index_help) @@ -181,9 +194,3 @@ class ToolBoxSearch(object): hits_with_score = sorted(hits_with_score.items(), key=lambda x: x[1], reverse=True) # Return the tool ids return [item[0] for item in hits_with_score[0:int(tool_search_limit)]] - - -def _temp_storage(self, name=None): - path = tempfile.mkdtemp() - tempstore = FileStorage(path) - return tempstore.create() diff --git a/lib/galaxy/webapps/galaxy/config_schema.yml b/lib/galaxy/webapps/galaxy/config_schema.yml index 6a01b393314..d4b6a2a96f7 100644 --- a/lib/galaxy/webapps/galaxy/config_schema.yml +++ b/lib/galaxy/webapps/galaxy/config_schema.yml @@ -759,6 +759,14 @@ mapping: Per tool_conf cache locations can be configured in (shed_)tool_conf.xml files using the tool_cache_data_dir attribute. + tool_search_index_dir: + type: str + default: tool_search_index + path_resolves_to: data_dir + required: false + desc: + Directory in which the toolbox search index is stored. + delay_tool_initialization: type: bool default: false diff --git a/lib/tool_shed/util/shed_index.py b/lib/tool_shed/util/shed_index.py index 5e537304b61..00b8eb5d59a 100644 --- a/lib/tool_shed/util/shed_index.py +++ b/lib/tool_shed/util/shed_index.py @@ -2,11 +2,11 @@ import logging import os from mercurial import hg, ui -from whoosh import index from whoosh.writing import AsyncWriter import tool_shed.webapp.model.mapping as ts_mapping from galaxy.tool_util.loader_directory import load_tool_elements_from_path +from galaxy.tools.search import get_or_create_index from galaxy.util import ( directory_hash_id, ExecutionTimer, @@ -21,24 +21,13 @@ from tool_shed.webapp.search.tool_search import schema as tool_schema log = logging.getLogger(__name__) -def get_or_create_index(whoosh_index_dir): +def _get_or_create_index(whoosh_index_dir): tool_index_dir = os.path.join(whoosh_index_dir, 'tools') if not os.path.exists(whoosh_index_dir): os.makedirs(whoosh_index_dir) if not os.path.exists(tool_index_dir): os.makedirs(tool_index_dir) - return _get_or_create_index(whoosh_index_dir, repo_schema), _get_or_create_index(tool_index_dir, tool_schema) - - -def _get_or_create_index(index_dir, schema): - if index.exists_in(index_dir): - idx = index.open_dir(index_dir) - try: - assert idx.schema == schema - return idx - except AssertionError: - log.warning("Index at '%s' uses outdated schema, creating new index", index_dir) - return index.create_in(index_dir, schema=schema) + return get_or_create_index(whoosh_index_dir, repo_schema), get_or_create_index(tool_index_dir, tool_schema) def build_index(whoosh_index_dir, file_path, hgweb_config_dir, dburi, **kwargs): @@ -50,7 +39,7 @@ def build_index(whoosh_index_dir, file_path, hgweb_config_dir, dburi, **kwargs): """ model = ts_mapping.init(file_path, dburi, engine_options={}, create_tables=False) sa_session = model.context.current - repo_index, tool_index = get_or_create_index(whoosh_index_dir) + repo_index, tool_index = _get_or_create_index(whoosh_index_dir) repo_index_writer = AsyncWriter(repo_index) tool_index_writer = AsyncWriter(tool_index) From 18c5b6a83945a644f87b6b53e416bf954b644aba Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 25 Apr 2020 19:40:30 +0200 Subject: [PATCH 32/33] Avoid passing None to delete_by_term and use with statement for AsyncWriter --- lib/galaxy/tools/search/__init__.py | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/galaxy/tools/search/__init__.py b/lib/galaxy/tools/search/__init__.py index 71482fa8302..9005dbc399c 100644 --- a/lib/galaxy/tools/search/__init__.py +++ b/lib/galaxy/tools/search/__init__.py @@ -78,18 +78,18 @@ class ToolBoxSearch(object): log.debug('Starting to build toolbox index.') self.index_count += 1 execution_timer = ExecutionTimer() - with self.index.searcher() as searcher: - indexed_tool_ids = {f.get('id') for f in searcher.all_stored_fields()} + with self.index.reader() as reader: + # Index ocasionally contains empty stored fields + indexed_tool_ids = {f['id'] for f in reader.all_stored_fields() if f} tool_ids_to_remove = (indexed_tool_ids - set(tool_cache._tool_paths_by_id.keys())).union(tool_cache._removed_tool_ids) - writer = AsyncWriter(self.index) - for tool_id in tool_ids_to_remove: - writer.delete_by_term('id', tool_id) - for tool_id in tool_cache._new_tool_ids - indexed_tool_ids: - tool = tool_cache.get_tool_by_id(tool_id) - if tool and tool.is_latest_version: - add_doc_kwds = self._create_doc(tool_id=tool_id, tool=tool, index_help=index_help) - writer.add_document(**add_doc_kwds) - writer.commit() + with AsyncWriter(self.index) as writer: + for tool_id in tool_ids_to_remove: + writer.delete_by_term('id', tool_id) + for tool_id in tool_cache._new_tool_ids - indexed_tool_ids: + tool = tool_cache.get_tool_by_id(tool_id) + if tool and tool.is_latest_version: + add_doc_kwds = self._create_doc(tool_id=tool_id, tool=tool, index_help=index_help) + writer.add_document(**add_doc_kwds) log.debug("Toolbox index finished %s", execution_timer) def _create_doc(self, tool_id, tool, index_help=True): From b0f1eb27d97fe72e7252b21f6b07b3879ce43c5e Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sun, 26 Apr 2020 13:09:20 +0200 Subject: [PATCH 33/33] Add new paths to config/test_config_values.py --- test/unit/config/test_config_values.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/unit/config/test_config_values.py b/test/unit/config/test_config_values.py index ae2720e4a7c..72432a0d863 100644 --- a/test/unit/config/test_config_values.py +++ b/test/unit/config/test_config_values.py @@ -93,10 +93,12 @@ class ExpectedValues: 'shed_tool_data_path': self._in_root_dir('tool-data'), 'shed_tool_data_table_config': self._in_managed_config_dir('shed_tool_data_table_conf.xml'), 'template_cache_path': self._in_data_dir('compiled_templates'), + 'tool_cache_data_dir': self._in_data_dir('tool_cache'), 'tool_config_file': self._in_sample_dir('tool_conf.xml.sample'), 'tool_data_path': self._in_root_dir('tool-data'), 'tool_data_table_config_path': self._in_sample_dir('tool_data_table_conf.xml.sample'), 'tool_path': self._in_root_dir('tools'), + 'tool_search_index_dir': self._in_data_dir('tool_search_index'), 'tool_sheds_config_file': self._in_config_dir('tool_sheds_conf.xml'), 'tool_test_data_directories': self._in_root_dir('test-data'), 'user_preferences_extra_conf_path': self._in_config_dir('user_preferences_extra_conf.yml'),