Merge pull request #3814 from mvdbeek/repository_updates

Awesome, thanks @mvdbeek !

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
 

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 )

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

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)

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

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")