From eddfec36fdf6d19bae398d09ee16952136d2b379 Mon Sep 17 00:00:00 2001 From: John Chilton Date: Sat, 26 Dec 2015 15:31:25 +0000 Subject: [PATCH] Refactor package_tool for better separation of concerns. The tool should know how to package itself, it shouldn't be the responsiblity of the toolbox. Update the code to reflect this and use more pythonic exception handling. --- lib/galaxy/tools/__init__.py | 105 ++++++++++++++++++++++ lib/galaxy/tools/toolbox/base.py | 108 +---------------------- lib/galaxy/web/base/controllers/admin.py | 18 ++-- lib/galaxy/webapps/galaxy/api/tools.py | 11 ++- 4 files changed, 122 insertions(+), 120 deletions(-) diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 32a3e799965..189bb3b7cba 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -7,6 +7,9 @@ import glob import json import logging import os +import re +import tarfile +import tempfile import threading import urllib from datetime import datetime @@ -1979,6 +1982,108 @@ class Tool( object, Dictifiable ): """ return output_collect.collect_dynamic_collections( self, output, **kwds ) + def to_archive(self): + tool = self.tool + tarball_files = [] + temp_files = [] + tool_xml = open( os.path.abspath( tool.config_file ), 'r' ).read() + # Retrieve tool help images and rewrite the tool's xml into a temporary file with the path + # modified to be relative to the repository root. + image_found = False + if tool.help is not None: + tool_help = tool.help._source + # Check each line of the rendered tool help for an image tag that points to a location under static/ + for help_line in tool_help.split( '\n' ): + image_regex = re.compile( 'img alt="[^"]+" src="\${static_path}/([^"]+)"' ) + matches = re.search( image_regex, help_line ) + if matches is not None: + tool_help_image = matches.group(1) + tarball_path = tool_help_image + filesystem_path = os.path.abspath( os.path.join( self.app.config.root, 'static', tool_help_image ) ) + if os.path.exists( filesystem_path ): + tarball_files.append( ( filesystem_path, tarball_path ) ) + image_found = True + tool_xml = tool_xml.replace( '${static_path}/%s' % tarball_path, tarball_path ) + # If one or more tool help images were found, add the modified tool XML to the tarball instead of the original. + if image_found: + fd, new_tool_config = tempfile.mkstemp( suffix='.xml' ) + os.close( fd ) + open( new_tool_config, 'w' ).write( tool_xml ) + tool_tup = ( os.path.abspath( new_tool_config ), os.path.split( tool.config_file )[-1] ) + temp_files.append( os.path.abspath( new_tool_config ) ) + else: + tool_tup = ( os.path.abspath( tool.config_file ), os.path.split( tool.config_file )[-1] ) + tarball_files.append( tool_tup ) + # TODO: This feels hacky. + tool_command = tool.command.strip().split()[0] + tool_path = os.path.dirname( os.path.abspath( tool.config_file ) ) + # Add the tool XML to the tuple that will be used to populate the tarball. + if os.path.exists( os.path.join( tool_path, tool_command ) ): + tarball_files.append( ( os.path.join( tool_path, tool_command ), tool_command ) ) + # Find and add macros and code files. + for external_file in tool.get_externally_referenced_paths( os.path.abspath( tool.config_file ) ): + external_file_abspath = os.path.abspath( os.path.join( tool_path, external_file ) ) + tarball_files.append( ( external_file_abspath, external_file ) ) + if os.path.exists( os.path.join( tool_path, "Dockerfile" ) ): + tarball_files.append( ( os.path.join( tool_path, "Dockerfile" ), "Dockerfile" ) ) + # Find tests, and check them for test data. + tests = tool.tests + if tests is not None: + for test in tests: + # Add input file tuples to the list. + for input in test.inputs: + for input_value in test.inputs[ input ]: + input_path = os.path.abspath( os.path.join( 'test-data', input_value ) ) + if os.path.exists( input_path ): + td_tup = ( input_path, os.path.join( 'test-data', input_value ) ) + tarball_files.append( td_tup ) + # And add output file tuples to the list. + for label, filename, _ in test.outputs: + output_filepath = os.path.abspath( os.path.join( 'test-data', filename ) ) + if os.path.exists( output_filepath ): + td_tup = ( output_filepath, os.path.join( 'test-data', filename ) ) + tarball_files.append( td_tup ) + for param in tool.input_params: + # Check for tool data table definitions. + if hasattr( param, 'options' ): + if hasattr( param.options, 'tool_data_table' ): + data_table = param.options.tool_data_table + if hasattr( data_table, 'filenames' ): + data_table_definitions = [] + for data_table_filename in data_table.filenames: + # FIXME: from_shed_config seems to always be False. + if not data_table.filenames[ data_table_filename ][ 'from_shed_config' ]: + tar_file = data_table.filenames[ data_table_filename ][ 'filename' ] + '.sample' + sample_file = os.path.join( data_table.filenames[ data_table_filename ][ 'tool_data_path' ], + tar_file ) + # Use the .sample file, if one exists. If not, skip this data table. + if os.path.exists( sample_file ): + tarfile_path, tarfile_name = os.path.split( tar_file ) + tarfile_path = os.path.join( 'tool-data', tarfile_name ) + tarball_files.append( ( sample_file, tarfile_path ) ) + data_table_definitions.append( data_table.xml_string ) + if len( data_table_definitions ) > 0: + # Put the data table definition XML in a temporary file. + table_definition = '\n\n %s' + table_definition = table_definition % '\n'.join( data_table_definitions ) + fd, table_conf = tempfile.mkstemp() + os.close( fd ) + open( table_conf, 'w' ).write( table_definition ) + tarball_files.append( ( table_conf, os.path.join( 'tool-data', 'tool_data_table_conf.xml.sample' ) ) ) + temp_files.append( table_conf ) + # Create the tarball. + fd, tarball_archive = tempfile.mkstemp( suffix='.tgz' ) + os.close( fd ) + tarball = tarfile.open( name=tarball_archive, mode='w:gz' ) + # Add the files from the previously generated list. + for fspath, tarpath in tarball_files: + tarball.add( fspath, arcname=tarpath ) + tarball.close() + # Delete any temporary files that were generated. + for temp_file in temp_files: + os.remove( temp_file ) + return tarball_archive + def to_dict( self, trans, link_details=False, io_details=False ): """ Returns dict of tool. """ diff --git a/lib/galaxy/tools/toolbox/base.py b/lib/galaxy/tools/toolbox/base.py index b1fd6335820..098874d3dd7 100644 --- a/lib/galaxy/tools/toolbox/base.py +++ b/lib/galaxy/tools/toolbox/base.py @@ -1,14 +1,13 @@ import logging import os -import re import string -import tarfile -import tempfile from markupsafe import escape from six.moves.urllib.parse import urlparse from six import iteritems +from galaxy.exceptions import ObjectNotFound + from galaxy.util.dictifiable import Dictifiable from galaxy.util.odict import odict @@ -763,109 +762,10 @@ class AbstractToolBox( Dictifiable, ManagesIntegratedToolPanelMixin, object ): """ # Make sure the tool is actually loaded. if tool_id not in self._tools_by_id: - return None, False, "No tool with id %s" % escape( tool_id ) + raise ObjectNotFound("No tool found with id '%s'." % escape( tool_id )) else: tool = self._tools_by_id[ tool_id ] - tarball_files = [] - temp_files = [] - tool_xml = open( os.path.abspath( tool.config_file ), 'r' ).read() - # Retrieve tool help images and rewrite the tool's xml into a temporary file with the path - # modified to be relative to the repository root. - image_found = False - if tool.help is not None: - tool_help = tool.help._source - # Check each line of the rendered tool help for an image tag that points to a location under static/ - for help_line in tool_help.split( '\n' ): - image_regex = re.compile( 'img alt="[^"]+" src="\${static_path}/([^"]+)"' ) - matches = re.search( image_regex, help_line ) - if matches is not None: - tool_help_image = matches.group(1) - tarball_path = tool_help_image - filesystem_path = os.path.abspath( os.path.join( trans.app.config.root, 'static', tool_help_image ) ) - if os.path.exists( filesystem_path ): - tarball_files.append( ( filesystem_path, tarball_path ) ) - image_found = True - tool_xml = tool_xml.replace( '${static_path}/%s' % tarball_path, tarball_path ) - # If one or more tool help images were found, add the modified tool XML to the tarball instead of the original. - if image_found: - fd, new_tool_config = tempfile.mkstemp( suffix='.xml' ) - os.close( fd ) - open( new_tool_config, 'w' ).write( tool_xml ) - tool_tup = ( os.path.abspath( new_tool_config ), os.path.split( tool.config_file )[-1] ) - temp_files.append( os.path.abspath( new_tool_config ) ) - else: - tool_tup = ( os.path.abspath( tool.config_file ), os.path.split( tool.config_file )[-1] ) - tarball_files.append( tool_tup ) - # TODO: This feels hacky. - tool_command = tool.command.strip().split()[0] - tool_path = os.path.dirname( os.path.abspath( tool.config_file ) ) - # Add the tool XML to the tuple that will be used to populate the tarball. - if os.path.exists( os.path.join( tool_path, tool_command ) ): - tarball_files.append( ( os.path.join( tool_path, tool_command ), tool_command ) ) - # Find and add macros and code files. - for external_file in tool.get_externally_referenced_paths( os.path.abspath( tool.config_file ) ): - external_file_abspath = os.path.abspath( os.path.join( tool_path, external_file ) ) - tarball_files.append( ( external_file_abspath, external_file ) ) - if os.path.exists( os.path.join( tool_path, "Dockerfile" ) ): - tarball_files.append( ( os.path.join( tool_path, "Dockerfile" ), "Dockerfile" ) ) - # Find tests, and check them for test data. - tests = tool.tests - if tests is not None: - for test in tests: - # Add input file tuples to the list. - for input in test.inputs: - for input_value in test.inputs[ input ]: - input_path = os.path.abspath( os.path.join( 'test-data', input_value ) ) - if os.path.exists( input_path ): - td_tup = ( input_path, os.path.join( 'test-data', input_value ) ) - tarball_files.append( td_tup ) - # And add output file tuples to the list. - for label, filename, _ in test.outputs: - output_filepath = os.path.abspath( os.path.join( 'test-data', filename ) ) - if os.path.exists( output_filepath ): - td_tup = ( output_filepath, os.path.join( 'test-data', filename ) ) - tarball_files.append( td_tup ) - for param in tool.input_params: - # Check for tool data table definitions. - if hasattr( param, 'options' ): - if hasattr( param.options, 'tool_data_table' ): - data_table = param.options.tool_data_table - if hasattr( data_table, 'filenames' ): - data_table_definitions = [] - for data_table_filename in data_table.filenames: - # FIXME: from_shed_config seems to always be False. - if not data_table.filenames[ data_table_filename ][ 'from_shed_config' ]: - tar_file = data_table.filenames[ data_table_filename ][ 'filename' ] + '.sample' - sample_file = os.path.join( data_table.filenames[ data_table_filename ][ 'tool_data_path' ], - tar_file ) - # Use the .sample file, if one exists. If not, skip this data table. - if os.path.exists( sample_file ): - tarfile_path, tarfile_name = os.path.split( tar_file ) - tarfile_path = os.path.join( 'tool-data', tarfile_name ) - tarball_files.append( ( sample_file, tarfile_path ) ) - data_table_definitions.append( data_table.xml_string ) - if len( data_table_definitions ) > 0: - # Put the data table definition XML in a temporary file. - table_definition = '\n\n %s' - table_definition = table_definition % '\n'.join( data_table_definitions ) - fd, table_conf = tempfile.mkstemp() - os.close( fd ) - open( table_conf, 'w' ).write( table_definition ) - tarball_files.append( ( table_conf, os.path.join( 'tool-data', 'tool_data_table_conf.xml.sample' ) ) ) - temp_files.append( table_conf ) - # Create the tarball. - fd, tarball_archive = tempfile.mkstemp( suffix='.tgz' ) - os.close( fd ) - tarball = tarfile.open( name=tarball_archive, mode='w:gz' ) - # Add the files from the previously generated list. - for fspath, tarpath in tarball_files: - tarball.add( fspath, arcname=tarpath ) - tarball.close() - # Delete any temporary files that were generated. - for temp_file in temp_files: - os.remove( temp_file ) - return tarball_archive, True, None - return None, False, "An unknown error occurred." + return tool.to_archive() def reload_tool_by_id( self, tool_id ): """ diff --git a/lib/galaxy/web/base/controllers/admin.py b/lib/galaxy/web/base/controllers/admin.py index 360e2c98907..57ffca7b485 100644 --- a/lib/galaxy/web/base/controllers/admin.py +++ b/lib/galaxy/web/base/controllers/admin.py @@ -68,26 +68,24 @@ class Admin( object ): def package_tool( self, trans, **kwd ): params = util.Params( kwd ) message = util.restore_text( params.get( 'message', '' ) ) - status = params.get( 'status', 'done' ) toolbox = self.app.toolbox tool_id = None if params.get( 'package_tool_button', False ): tool_id = params.get('tool_id', None) - tool_tarball, success, message = trans.app.toolbox.package_tool( trans, tool_id ) - if success: + try: + tool_tarball = trans.app.toolbox.package_tool( trans, tool_id ) trans.response.set_content_type( 'application/x-gzip' ) download_file = open( tool_tarball ) os.unlink( tool_tarball ) tarball_path, filename = os.path.split( tool_tarball ) trans.response.headers[ "Content-Disposition" ] = 'attachment; filename="%s.tgz"' % ( tool_id ) return download_file - else: - status = 'error' - return trans.fill_template( '/admin/package_tool.mako', - tool_id=tool_id, - toolbox=toolbox, - message=message, - status=status ) + except Exception: + return trans.fill_template( '/admin/package_tool.mako', + tool_id=tool_id, + toolbox=toolbox, + message=message, + status='error' ) @web.expose @web.require_admin diff --git a/lib/galaxy/webapps/galaxy/api/tools.py b/lib/galaxy/webapps/galaxy/api/tools.py index e4a3112d07e..dfa2febb1e8 100644 --- a/lib/galaxy/webapps/galaxy/api/tools.py +++ b/lib/galaxy/webapps/galaxy/api/tools.py @@ -198,12 +198,11 @@ class ToolsController( BaseAPIController, UsesVisualizationMixin ): @web.expose_api_raw @web.require_admin def download( self, trans, id, **kwds ): - tool_tarball, success, message = trans.app.toolbox.package_tool( trans, id ) - if success: - trans.response.set_content_type( 'application/x-gzip' ) - download_file = open( tool_tarball ) - trans.response.headers[ "Content-Disposition" ] = 'attachment; filename="%s.tgz"' % ( id ) - return download_file + tool_tarball = trans.app.toolbox.package_tool(trans, id) + trans.response.set_content_type('application/x-gzip') + download_file = open(tool_tarball, "rb") + trans.response.headers[ "Content-Disposition" ] = 'attachment; filename="%s.tgz"' % (id) + return download_file @expose_api_anonymous def create( self, trans, payload, **kwd ):