From 197aa402d72895d370307ce9724c86b892cded96 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Sat, 25 Mar 2017 12:03:53 +0100 Subject: [PATCH] Improve ToolCache cleanup logic Pull `removed_tool_ids` out of try/except, so that we can return `removed_tool_ids` even if an exception occured. Fix a typo in the `_should_cleanup` docstring. Many thanks for the suggestions @nsoranzo. --- lib/galaxy/tools/toolbox/cache.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/galaxy/tools/toolbox/cache.py b/lib/galaxy/tools/toolbox/cache.py index 5dbf7d2fcae..22b1fc882c6 100644 --- a/lib/galaxy/tools/toolbox/cache.py +++ b/lib/galaxy/tools/toolbox/cache.py @@ -1,5 +1,4 @@ import os -import time from galaxy.util.hash_util import md5_hash_file @@ -22,28 +21,28 @@ class ToolCache(object): 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)} - removed_tool_ids = [] for config_filename, tool_ids in paths_to_cleanup.items(): - removed_tool_ids.extend(tool_ids) 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] - return removed_tool_ids + 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. - return [] + pass + return removed_tool_ids def _should_cleanup(self, config_filename): - """Return True of `config_filename` does not exist or if modtime and hash have changes, else return False.""" + """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 = time.ctime(os.path.getmtime(config_filename)) - if self._mod_time_by_path.get(config_filename) != new_mtime: + 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 @@ -65,6 +64,6 @@ class ToolCache(object): 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] = time.ctime(os.path.getmtime(config_filename)) + 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