diff --git a/lib/galaxy/queue_worker.py b/lib/galaxy/queue_worker.py index f3f8245b7e9..167be3e2339 100644 --- a/lib/galaxy/queue_worker.py +++ b/lib/galaxy/queue_worker.py @@ -87,7 +87,8 @@ def reload_tool(app, **kwargs): def reload_toolbox(app, **kwargs): log.debug("Executing toolbox reload on '%s'", app.config.server_name) reload_count = app.toolbox._reload_count - app.tool_cache.cleanup() + if app.tool_cache: + app.tool_cache.cleanup() app.toolbox = _get_new_toolbox(app) app.toolbox._reload_count = reload_count + 1 diff --git a/lib/galaxy/tools/toolbox/base.py b/lib/galaxy/tools/toolbox/base.py index 402e490bb47..28a73fa4ab0 100644 --- a/lib/galaxy/tools/toolbox/base.py +++ b/lib/galaxy/tools/toolbox/base.py @@ -79,7 +79,13 @@ class AbstractToolBox( Dictifiable, ManagesIntegratedToolPanelMixin, object ): if tool_conf_watcher: self._tool_conf_watcher = tool_conf_watcher # Avoids (re-)starting threads in uwsgi else: - self._tool_conf_watcher = get_tool_conf_watcher(lambda: self.handle_reload_toolbox()) + if hasattr(self.app, 'tool_cache'): + # Normal galaxy instances should have a tool_cache, + # but the toolshed does not. + tool_cache = self.app.tool_cache + else: + tool_cache = None + self._tool_conf_watcher = get_tool_conf_watcher(reload_callback=lambda: self.handle_reload_toolbox(), tool_cache=tool_cache) self._filter_factory = FilterFactory( self ) self._tool_tag_manager = tool_tag_manager( app ) self._init_tools_from_configs( config_filenames ) diff --git a/lib/galaxy/tools/toolbox/cache.py b/lib/galaxy/tools/toolbox/cache.py index ee576d1d865..22b1fc882c6 100644 --- a/lib/galaxy/tools/toolbox/cache.py +++ b/lib/galaxy/tools/toolbox/cache.py @@ -13,16 +13,39 @@ class ToolCache(object): self._hash_by_tool_paths = {} self._tools_by_path = {} self._tool_paths_by_id = {} + self._mod_time_by_path = {} def cleanup(self): - """Remove uninstalled tools from tool cache if they are not on disk anymore or if their content has changed.""" - paths_to_cleanup = {path: tool.all_ids for path, tool in self._tools_by_path.items() if not os.path.exists(path) or md5_hash_file(path) != self._hash_by_tool_paths[path]} - for config_filename, tool_ids in paths_to_cleanup.items(): - del self._hash_by_tool_paths[config_filename] - del self._tools_by_path[config_filename] - for tool_id in tool_ids: - if tool_id in self._tool_paths_by_id: - del self._tool_paths_by_id[tool_id] + """ + Remove uninstalled tools from tool cache if they are not on disk anymore or if their content has changed. + + Returns list of tool_ids that have been removed. + """ + removed_tool_ids = [] + try: + 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(): + del self._hash_by_tool_paths[config_filename] + del self._tools_by_path[config_filename] + for tool_id in tool_ids: + if tool_id in self._tool_paths_by_id: + del self._tool_paths_by_id[tool_id] + removed_tool_ids.extend(tool_ids) + except Exception: + # If by chance the file is being removed while calculating the hash or modtime + # we don't want the thread to die. + pass + return removed_tool_ids + + def _should_cleanup(self, config_filename): + """Return True if `config_filename` does not exist or if modtime and hash have changes, else return False.""" + 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): + return True + return False def get_tool(self, config_filename): """ Get the tool from the cache if the tool is up to date. @@ -35,10 +58,12 @@ 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] def cache_tool(self, config_filename, tool): tool_hash = md5_hash_file(config_filename) tool_id = str( tool.id ) self._hash_by_tool_paths[config_filename] = tool_hash + self._mod_time_by_path[config_filename] = os.path.getmtime(config_filename) self._tool_paths_by_id[tool_id] = config_filename self._tools_by_path[config_filename] = tool diff --git a/lib/galaxy/tools/toolbox/watcher.py b/lib/galaxy/tools/toolbox/watcher.py index 862b30017ee..cbd9fa75a4f 100644 --- a/lib/galaxy/tools/toolbox/watcher.py +++ b/lib/galaxy/tools/toolbox/watcher.py @@ -48,8 +48,8 @@ def get_observer_class(config_value, default, monitor_what_str): return observer_class -def get_tool_conf_watcher(reload_callback): - return ToolConfWatcher(reload_callback) +def get_tool_conf_watcher(reload_callback, tool_cache=None): + return ToolConfWatcher(reload_callback=reload_callback, tool_cache=tool_cache) def get_tool_data_dir_watcher(tool_data_tables, config): @@ -73,8 +73,9 @@ def get_tool_watcher(toolbox, config): class ToolConfWatcher(object): - def __init__(self, reload_callback): + def __init__(self, reload_callback, tool_cache=None): self.paths = {} + self.cache = tool_cache self._active = False self._lock = threading.Lock() self.thread = threading.Thread(target=self.check, name="ToolConfWatcher.thread") @@ -92,6 +93,7 @@ class ToolConfWatcher(object): self.thread.join() def check(self): + """Check for changes in self.paths or self.cache and call the event handler.""" hashes = { key: None for key in self.paths.keys() } while self._active: do_reload = False @@ -113,7 +115,10 @@ class ToolConfWatcher(object): hashes[path] = new_hash log.debug("The file '%s' has changes.", path) do_reload = True - + if not do_reload and self.cache: + removed_ids = self.cache.cleanup() + if removed_ids: + do_reload = True if do_reload: with self._lock: t = threading.Thread(target=self.event_handler.on_any_event) diff --git a/test/unit/tools/test_toolbox.py b/test/unit/tools/test_toolbox.py index 6e9a2729fea..70a50f7f246 100644 --- a/test/unit/tools/test_toolbox.py +++ b/test/unit/tools/test_toolbox.py @@ -57,6 +57,7 @@ class BaseToolBoxTestCase( unittest.TestCase, tools_support.UsesApp, tools_supp self.reindexed = False self.setup_app( mock_model=False ) install_model = mapping.init( "sqlite:///:memory:", create_tables=True ) + self.app.tool_cache = None self.app.install_model = install_model self.app.reindex_tool_search = self.__reindex itp_config = os.path.join(self.test_directory, "integrated_tool_panel.xml") @@ -429,6 +430,7 @@ class SimplifiedToolBox( ToolBox ): def __init__( self, test_case ): app = test_case.app # Handle app/config stuff needed by toolbox but not by tools. + app.tool_cache = None app.job_config.get_tool_resource_parameters = lambda tool_id: None app.config.update_integrated_tool_panel = True config_files = test_case.config_files diff --git a/test/unit/tools/test_watcher.py b/test/unit/tools/test_watcher.py index 7054856dc48..b2f3dce1fd7 100644 --- a/test/unit/tools/test_watcher.py +++ b/test/unit/tools/test_watcher.py @@ -40,6 +40,7 @@ def test_tool_conf_watcher(): with __test_directory() as t: tool_conf_path = path.join(t, "test_conf.xml") + open(tool_conf_path, "w").write("a") conf_watcher.watch_file(tool_conf_path) time.sleep(1) open(tool_conf_path, "w").write("b")