From cf2645f9a658c5e122fea0309bf73a1f64590c39 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Mon, 29 Oct 2018 10:34:34 +0100 Subject: [PATCH] Fix toolshed error in display_tool if tool is invalid Fixes the following exception: ``` AttributeError: 'NoneType' object has no attribute 'id' File "galaxy/web/framework/middleware/sentry.py", line 43, in __call__ iterable = self.application(environ, start_response) File "/srv/toolshed/main/venv/local/lib/python2.7/site-packages/paste/translogger.py", line 69, in __call__ return self.application(environ, replacement_start_response) File "/srv/toolshed/main/venv/local/lib/python2.7/site-packages/paste/recursive.py", line 85, in __call__ return self.application(environ, start_response) File "routes/middleware.py", line 141, in __call__ response = self.app(environ, start_response) File "/srv/toolshed/main/venv/local/lib/python2.7/site-packages/paste/httpexceptions.py", line 640, in __call__ return self.application(environ, start_response) File "galaxy/web/framework/base.py", line 143, in __call__ return self.handle_request(environ, start_response) File "galaxy/web/framework/base.py", line 222, in handle_request body = method(trans, **kwargs) File "galaxy/webapps/tool_shed/controllers/repository.py", line 888, in display_tool tool_state = tool_util.new_state(trans, tool, invalid=False) File "tool_shed/util/tool_util.py", line 211, in new_state log.debug('Failed to build tool state for tool "%s" using standard method, will try to fall back on custom method: %s', tool.id, e) ``` (from https://sentry.galaxyproject.org/sentry/toolshed/issues/279820/). Might also fix some other issues when redering workflow svg for workflows with invalid tools. --- lib/galaxy/webapps/tool_shed/api/tools.py | 8 ++++---- .../tool_shed/controllers/repository.py | 9 ++++----- lib/tool_shed/tools/tool_validator.py | 20 +++++++++---------- lib/tool_shed/util/workflow_util.py | 8 ++++---- 4 files changed, 21 insertions(+), 24 deletions(-) diff --git a/lib/galaxy/webapps/tool_shed/api/tools.py b/lib/galaxy/webapps/tool_shed/api/tools.py index e22274b0ceb..8aaf50d19b6 100644 --- a/lib/galaxy/webapps/tool_shed/api/tools.py +++ b/lib/galaxy/webapps/tool_shed/api/tools.py @@ -166,10 +166,10 @@ class ToolsController(BaseAPIController): with ValidationContext.from_app(trans.app) as validation_context: tv = tool_validator.ToolValidator(validation_context) - repository, tool, message = tv.load_tool_from_changeset_revision(tsr_id, - changeset, - found_tool.tool_config) - if message: + repository, tool, valid, message = tv.load_tool_from_changeset_revision(tsr_id, + changeset, + found_tool.tool_config) + if message or not valid: status = 'error' return dict(message=message, status=status) tool_help = '' diff --git a/lib/galaxy/webapps/tool_shed/controllers/repository.py b/lib/galaxy/webapps/tool_shed/controllers/repository.py index bd74cff3699..52e75562929 100644 --- a/lib/galaxy/webapps/tool_shed/controllers/repository.py +++ b/lib/galaxy/webapps/tool_shed/controllers/repository.py @@ -875,17 +875,16 @@ class RepositoryController(BaseUIController, ratings_util.ItemRatings): @web.expose def display_tool(self, trans, repository_id, tool_config, changeset_revision, **kwd): - message = escape(kwd.get('message', '')) status = kwd.get('status', 'done') render_repository_actions_for = kwd.get('render_repository_actions_for', 'tool_shed') with ValidationContext.from_app(trans.app) as validation_context: tv = tool_validator.ToolValidator(validation_context) - repository, tool, message = tv.load_tool_from_changeset_revision(repository_id, + repository, tool, valid, message = tv.load_tool_from_changeset_revision(repository_id, changeset_revision, tool_config) - if message: + if message or not valid: status = 'error' - tool_state = tool_util.new_state(trans, tool, invalid=False) + tool_state = tool_util.new_state(trans, tool, invalid=not valid) metadata = metadata_util.get_repository_metadata_by_repository_id_changeset_revision(trans.app, repository_id, changeset_revision, @@ -1772,7 +1771,7 @@ class RepositoryController(BaseUIController, ratings_util.ItemRatings): with ValidationContext.from_app(trans.app) as validation_context: tv = tool_validator.ToolValidator(validation_context) - repository, tool, error_message = tv.load_tool_from_changeset_revision(repository_id, + repository, tool, valid, error_message = tv.load_tool_from_changeset_revision(repository_id, changeset_revision, tool_config) tool_state = tool_util.new_state(trans, tool, invalid=True) diff --git a/lib/tool_shed/tools/tool_validator.py b/lib/tool_shed/tools/tool_validator.py index 0abfdadea2c..5302e45759c 100644 --- a/lib/tool_shed/tools/tool_validator.py +++ b/lib/tool_shed/tools/tool_validator.py @@ -202,7 +202,6 @@ class ToolValidator(object): """ message = '' sample_files = self.copy_disk_sample_files_to_dir(repo_files_dir, work_dir) - tool_data_table_config = None if sample_files: if 'tool_data_table_conf.xml.sample' in sample_files: # Load entries into the tool_data_tables if the tool requires them. @@ -216,6 +215,7 @@ class ToolValidator(object): def handle_sample_files_and_load_tool_from_tmp_config(self, repo, repository_id, changeset_revision, tool_config_filename, work_dir): tool = None + valid = False message = '' # We're not currently doing anything with the returned list of deleted_sample_files here. It is # intended to help handle sample files that are in the manifest, but have been deleted from disk. @@ -224,16 +224,13 @@ class ToolValidator(object): if 'tool_data_table_conf.xml.sample' in sample_files: # Load entries into the tool_data_tables if the tool requires them. tool_data_table_config = os.path.join(work_dir, 'tool_data_table_conf.xml') - if tool_data_table_config: - error, message = self.stdtm.handle_sample_tool_data_table_conf_file(tool_data_table_config, - persist=False) - if error: - log.debug(message) + error, message = self.stdtm.handle_sample_tool_data_table_conf_file(tool_data_table_config, + persist=False) manifest_ctx, ctx_file = hg_util.get_ctx_file_path_from_manifest(tool_config_filename, repo, changeset_revision) if manifest_ctx and ctx_file: - tool, message2 = self.load_tool_from_tmp_config(repo, repository_id, manifest_ctx, ctx_file, work_dir) + tool, valid, message2 = self.load_tool_from_tmp_config(repo, repository_id, manifest_ctx, ctx_file, work_dir) message = self.concat_messages(message, message2) - return tool, message, sample_files + return tool, valid, message, sample_files def load_tool_from_changeset_revision(self, repository_id, changeset_revision, tool_config_filename): """ @@ -273,7 +270,7 @@ class ToolValidator(object): displaying_invalid_tool=True) message = self.concat_messages(message, message2) else: - tool, message, sample_files = \ + tool, valid, message, sample_files = \ self.handle_sample_files_and_load_tool_from_tmp_config(repo, repository_id, changeset_revision, @@ -282,7 +279,7 @@ class ToolValidator(object): basic_util.remove_dir(work_dir) # Reset the tool_data_tables by loading the empty tool_data_table_conf.xml file. self.stdtm.reset_tool_data_tables() - return repository, tool, message + return repository, tool, valid, message def load_tool_from_config(self, repository_id, full_path): tool_source = get_tool_source( @@ -308,6 +305,7 @@ class ToolValidator(object): def load_tool_from_tmp_config(self, repo, repository_id, ctx, ctx_file, work_dir): tool = None + valid = False message = '' tmp_tool_config = hg_util.get_named_tmpfile_from_ctx(ctx, ctx_file, work_dir) if tmp_tool_config: @@ -332,4 +330,4 @@ class ToolValidator(object): os.unlink(tmp_tool_config) except Exception: pass - return tool, message + return tool, valid, message diff --git a/lib/tool_shed/util/workflow_util.py b/lib/tool_shed/util/workflow_util.py index 0604759c040..9de41eb748b 100644 --- a/lib/tool_shed/util/workflow_util.py +++ b/lib/tool_shed/util/workflow_util.py @@ -43,10 +43,10 @@ class RepoToolModule(ToolModule): tv = tool_validator.ToolValidator(validation_context) for tool_dict in tools_metadata: if self.tool_id in [tool_dict['id'], tool_dict['guid']]: - repository, self.tool, message = tv.load_tool_from_changeset_revision(repository_id, - changeset_revision, - tool_dict['tool_config']) - if message and self.tool is None: + repository, self.tool, valid, message = tv.load_tool_from_changeset_revision(repository_id, + changeset_revision, + tool_dict['tool_config']) + if self.tool is None and message or not valid: self.errors = 'unavailable' break else: