commit/galaxy-central: greg: Improve exception handling when loading and displaying tools in the tool shed, check content of files uploaded to a tool shed repository only when necessary, and clarify that borwsing repository files is only available for the repository tip.
1 new commit in galaxy-central: https://bitbucket.org/galaxy/galaxy-central/changeset/537a42e5d265/ changeset: 537a42e5d265 user: greg date: 2011-11-16 17:03:14 summary: Improve exception handling when loading and displaying tools in the tool shed, check content of files uploaded to a tool shed repository only when necessary, and clarify that borwsing repository files is only available for the repository tip. affected #: 14 files diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 lib/galaxy/webapps/community/controllers/common.py --- a/lib/galaxy/webapps/community/controllers/common.py +++ b/lib/galaxy/webapps/community/controllers/common.py @@ -380,17 +380,19 @@ for name in files: # Find all tool configs. if name != 'datatypes_conf.xml' and name.endswith( '.xml' ): + full_path = os.path.abspath( os.path.join( root, name ) ) try: - full_path = os.path.abspath( os.path.join( root, name ) ) tool = load_tool( trans, full_path ) - if tool is not None: - can_set_metadata, invalid_files = check_tool_input_params( trans, name, tool, sample_files, invalid_files ) - if can_set_metadata: - # Update the list of metadata dictionaries for tools in metadata_dict. - tool_config = os.path.join( root, name ) - metadata_dict = generate_tool_metadata( trans, id, changeset_revision, tool_config, tool, metadata_dict ) + valid = True except Exception, e: + valid = False invalid_files.append( ( name, str( e ) ) ) + if valid and tool is not None: + can_set_metadata, invalid_files = check_tool_input_params( trans, name, tool, sample_files, invalid_files ) + if can_set_metadata: + # Update the list of metadata dictionaries for tools in metadata_dict. + tool_config = os.path.join( root, name ) + metadata_dict = generate_tool_metadata( trans, id, changeset_revision, tool_config, tool, metadata_dict ) # Find all exported workflows elif name.endswith( '.ga' ): try: @@ -427,17 +429,19 @@ fh.close() try: tool = load_tool( trans, tmp_filename ) - if tool is not None: - can_set_metadata, invalid_files = check_tool_input_params( trans, filename, tool, sample_files, invalid_files ) - if can_set_metadata: - # Update the list of metadata dictionaries for tools in metadata_dict. Note that filename - # here is the relative path to the config file within the change set context, something - # like filtering.xml, but when the change set was the repository tip, the value was - # something like database/community_files/000/repo_1/filtering.xml. This shouldn't break - # anything, but may result in a bit of confusion when maintaining the code / data over time. - metadata_dict = generate_tool_metadata( trans, id, changeset_revision, filename, tool, metadata_dict ) + valid = True except Exception, e: - invalid_files.append( ( name, str( e ) ) ) + invalid_files.append( ( filename, str( e ) ) ) + valid = False + if valid and tool is not None: + can_set_metadata, invalid_files = check_tool_input_params( trans, filename, tool, sample_files, invalid_files ) + if can_set_metadata: + # Update the list of metadata dictionaries for tools in metadata_dict. Note that filename + # here is the relative path to the config file within the change set context, something + # like filtering.xml, but when the change set was the repository tip, the value was + # something like database/community_files/000/repo_1/filtering.xml. This shouldn't break + # anything, but may result in a bit of confusion when maintaining the code / data over time. + metadata_dict = generate_tool_metadata( trans, id, changeset_revision, filename, tool, metadata_dict ) try: os.unlink( tmp_filename ) except: @@ -575,6 +579,17 @@ util.send_mail( frm, to, subject, body, trans.app.config ) except Exception, e: log.exception( "An error occurred sending a tool shed repository update alert by email." ) +def check_file_contents( trans ): + # See if any admin users have chosen to receive email alerts when a repository is updated. + # If so, the file contents of the update must be checked for inappropriate content. + admin_users = trans.app.config.get( "admin_users", "" ).split( "," ) + for repository in trans.sa_session.query( trans.model.Repository ) \ + .filter( trans.model.Repository.table.c.email_alerts != None ): + email_alerts = from_json_string( repository.email_alerts ) + for user_email in email_alerts: + if user_email in admin_users: + return True + return False def update_for_browsing( trans, repository, current_working_dir, commit_message='' ): # Make a copy of a repository's files for browsing, remove from disk all files that # are not tracked, and commit all added, modified or removed files that have not yet diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 lib/galaxy/webapps/community/controllers/repository.py --- a/lib/galaxy/webapps/community/controllers/repository.py +++ b/lib/galaxy/webapps/community/controllers/repository.py @@ -773,7 +773,7 @@ tool_guids = [] for filename in ctx: # Find all tool configs in this repository changeset_revision. - if filename.endswith( '.xml' ): + if filename != 'datatypes_conf.xml' and filename.endswith( '.xml' ): fctx = ctx[ filename ] # Write the contents of the old tool config to a temporary file. fh = tempfile.NamedTemporaryFile( 'w' ) @@ -784,11 +784,11 @@ fh.close() try: tool = load_tool( trans, tmp_filename ) - if tool is not None: - tool_guids.append( generate_tool_guid( trans, repository, tool ) ) + valid = True except: - # File must not be a valid tool config even though it has a .xml extension. - pass + valid = False + if valid and tool is not None: + tool_guids.append( generate_tool_guid( trans, repository, tool ) ) try: os.unlink( tmp_filename ) except: @@ -1614,39 +1614,49 @@ status = params.get( 'status', 'done' ) webapp = params.get( 'webapp', 'community' ) repository = get_repository( trans, repository_id ) - repo = hg.repository( get_configured_ui(), repository.repo_path ) + repo = hg.repository( get_configured_ui(), repository.repo_path ) + valid = True + if changeset_revision == repository.tip: + try: + tool = load_tool( trans, os.path.abspath( tool_config ) ) + except Exception, e: + tool = None + valid = False + message = "Error loading tool: %s. Clicking <b>Reset metadata</b> may correct this error." % str( e ) + else: + # Get the tool config file name from the hgweb url, something like: + # /repos/test/convert_chars1/file/e58dcf0026c7/convert_characters.xml + old_tool_config_file_name = tool_config.split( '/' )[ -1 ] + ctx = get_changectx_for_changeset( trans, repo, changeset_revision ) + fctx = None + for filename in ctx: + filename_head, filename_tail = os.path.split( filename ) + if filename_tail == old_tool_config_file_name: + fctx = ctx[ filename ] + break + if fctx: + # Write the contents of the old tool config to a temporary file. + fh = tempfile.NamedTemporaryFile( 'w' ) + tmp_filename = fh.name + fh.close() + fh = open( tmp_filename, 'w' ) + fh.write( fctx.data() ) + fh.close() + try: + tool = load_tool( trans, tmp_filename ) + except Exception, e: + tool = None + valid = False + message = "Error loading tool: %s. Clicking <b>Reset metadata</b> may correct this error." % str( e ) + try: + os.unlink( tmp_filename ) + except: + pass + else: + tool = None + tool_state = self.__new_state( trans ) + is_malicious = change_set_is_malicious( trans, repository_id, repository.tip ) try: - if changeset_revision == repository.tip: - # Get the tool config from the file system we use for browsing. - tool = load_tool( trans, os.path.abspath( tool_config ) ) - else: - # Get the tool config file name from the hgweb url, something like: - # /repos/test/convert_chars1/file/e58dcf0026c7/convert_characters.xml - old_tool_config_file_name = tool_config.split( '/' )[ -1 ] - ctx = get_changectx_for_changeset( trans, repo, changeset_revision ) - fctx = None - for filename in ctx: - filename_head, filename_tail = os.path.split( filename ) - if filename_tail == old_tool_config_file_name: - fctx = ctx[ filename ] - break - if fctx: - # Write the contents of the old tool config to a temporary file. - fh = tempfile.NamedTemporaryFile( 'w' ) - tmp_filename = fh.name - fh.close() - fh = open( tmp_filename, 'w' ) - fh.write( fctx.data() ) - fh.close() - tool = load_tool( trans, tmp_filename ) - try: - os.unlink( tmp_filename ) - except: - pass - else: - tool = None - tool_state = self.__new_state( trans ) - is_malicious = change_set_is_malicious( trans, repository_id, repository.tip ) return trans.fill_template( "/webapps/community/repository/tool_form.mako", repository=repository, changeset_revision=changeset_revision, @@ -1657,7 +1667,7 @@ message=message, status=status ) except Exception, e: - message = "Error loading tool: %s. Click <b>Reset metadata</b> to correct this error." % str( e ) + message = "Error displaying tool, probably due to a problem in the tool config. The exception is: %s." % str( e ) if webapp == 'galaxy': return trans.response.send_redirect( web.url_for( controller='repository', action='preview_tools_in_changeset', diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 lib/galaxy/webapps/community/controllers/upload.py --- a/lib/galaxy/webapps/community/controllers/upload.py +++ b/lib/galaxy/webapps/community/controllers/upload.py @@ -103,7 +103,10 @@ full_path = os.path.abspath( os.path.join( repo_dir, uploaded_file_filename ) ) # Move the uploaded file to the load_point within the repository hierarchy. shutil.move( uploaded_file_name, full_path ) - if os.path.isfile( full_path ): + # See if any admin users have chosen to receive email alerts when a repository is + # updated. If so, check every uploaded file to ensure content is appropriate. + check_contents = check_file_contents( trans ) + if check_contents and os.path.isfile( full_path ): content_alert_str = self.__check_file_content( full_path ) else: content_alert_str = '' @@ -238,9 +241,12 @@ except OSError, e: # The directory is not empty pass + # See if any admin users have chosen to receive email alerts when a repository is + # updated. If so, check every uploaded file to ensure content is appropriate. + check_contents = check_file_contents( trans ) for filename_in_archive in filenames_in_archive: # Check file content to ensure it is appropriate. - if os.path.isfile( filename_in_archive ): + if check_contents and os.path.isfile( filename_in_archive ): content_alert_str += self.__check_file_content( filename_in_archive ) commands.add( repo.ui, repo, filename_in_archive ) if filename_in_archive.endswith( 'tool_data_table_conf.xml.sample' ): diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 templates/webapps/community/repository/browse_repository.mako --- a/templates/webapps/community/repository/browse_repository.mako +++ b/templates/webapps/community/repository/browse_repository.mako @@ -97,7 +97,7 @@ %if can_browse_contents: <div class="toolForm"> - <div class="toolFormTitle">Browse ${repository.name}</div> + <div class="toolFormTitle">Browse ${repository.name} revision ${repository.tip} (repository tip)</div> %if can_download: <div class="form-row"><label>Clone this repository:</label> diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 templates/webapps/community/repository/contact_owner.mako --- a/templates/webapps/community/repository/contact_owner.mako +++ b/templates/webapps/community/repository/contact_owner.mako @@ -13,9 +13,9 @@ can_manage = is_admin or repository.user == trans.user can_view_change_log = not is_new if can_push: - browse_label = 'Browse or delete repository files' + browse_label = 'Browse or delete repository tip files' else: - browse_label = 'Browse repository files' + browse_label = 'Browse repository tip files' %><%! diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 templates/webapps/community/repository/manage_repository.mako --- a/templates/webapps/community/repository/manage_repository.mako +++ b/templates/webapps/community/repository/manage_repository.mako @@ -16,9 +16,9 @@ can_rate = not is_new and trans.user and repository.user != trans.user can_view_change_log = not is_new if can_push: - browse_label = 'Browse or delete repository files' + browse_label = 'Browse or delete repository tip files' else: - browse_label = 'Browse repository files' + browse_label = 'Browse repository tip files' can_set_malicious = metadata and can_set_metadata and is_admin and changeset_revision == repository.tip %> diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 templates/webapps/community/repository/preview_tools_in_changeset.mako --- a/templates/webapps/community/repository/preview_tools_in_changeset.mako +++ b/templates/webapps/community/repository/preview_tools_in_changeset.mako @@ -13,9 +13,9 @@ can_browse_contents = not is_new can_view_change_log = not is_new if can_push: - browse_label = 'Browse or delete repository files' + browse_label = 'Browse or delete repository tip files' else: - browse_label = 'Browse repository files' + browse_label = 'Browse repository tip files' %><%! diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 templates/webapps/community/repository/rate_repository.mako --- a/templates/webapps/community/repository/rate_repository.mako +++ b/templates/webapps/community/repository/rate_repository.mako @@ -16,9 +16,9 @@ can_manage = is_admin or repository.user == trans.user can_view_change_log = not is_new if can_push: - browse_label = 'Browse or delete repository files' + browse_label = 'Browse or delete repository tip files' else: - browse_label = 'Browse repository files' + browse_label = 'Browse repository tip files' %><%! diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 templates/webapps/community/repository/tool_form.mako --- a/templates/webapps/community/repository/tool_form.mako +++ b/templates/webapps/community/repository/tool_form.mako @@ -18,9 +18,9 @@ can_manage = is_admin or repository.user == trans.user can_view_change_log = not is_new if can_push: - browse_label = 'Browse or delete repository files' + browse_label = 'Browse or delete repository tip files' else: - browse_label = 'Browse repository files' + browse_label = 'Browse repository tip files' %><html> diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 templates/webapps/community/repository/view_changelog.mako --- a/templates/webapps/community/repository/view_changelog.mako +++ b/templates/webapps/community/repository/view_changelog.mako @@ -15,9 +15,9 @@ can_upload = can_push can_download = not is_new and ( not is_malicious or can_push ) if can_push: - browse_label = 'Browse or delete repository files' + browse_label = 'Browse or delete repository tip files' else: - browse_label = 'Browse repository files' + browse_label = 'Browse repository tip files' %><%! diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 templates/webapps/community/repository/view_changeset.mako --- a/templates/webapps/community/repository/view_changeset.mako +++ b/templates/webapps/community/repository/view_changeset.mako @@ -16,9 +16,9 @@ can_upload = can_push can_download = not is_new and ( not is_malicious or can_push ) if can_push: - browse_label = 'Browse or delete repository files' + browse_label = 'Browse or delete repository tip files' else: - browse_label = 'Browse repository files' + browse_label = 'Browse repository tip files' %><%! diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 templates/webapps/community/repository/view_repository.mako --- a/templates/webapps/community/repository/view_repository.mako +++ b/templates/webapps/community/repository/view_repository.mako @@ -14,9 +14,9 @@ can_browse_contents = webapp == 'community' and not is_new can_view_change_log = webapp == 'community' and not is_new if can_push: - browse_label = 'Browse or delete repository files' + browse_label = 'Browse or delete repository tip files' else: - browse_label = 'Browse repository files' + browse_label = 'Browse repository tip files' %><%! diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 templates/webapps/community/repository/view_tool_metadata.mako --- a/templates/webapps/community/repository/view_tool_metadata.mako +++ b/templates/webapps/community/repository/view_tool_metadata.mako @@ -17,9 +17,9 @@ can_manage = is_admin or repository.user == trans.user can_view_change_log = webapp == 'community' and not is_new if can_push: - browse_label = 'Browse or delete repository files' + browse_label = 'Browse or delete repository tip files' else: - browse_label = 'Browse repository files' + browse_label = 'Browse repository tip files' %><%! diff -r 663a2a57ed4db4303661f3771c518b7611f9c85f -r 537a42e5d26578413f20b369643110404502b7c3 templates/webapps/community/repository/view_workflow.mako --- a/templates/webapps/community/repository/view_workflow.mako +++ b/templates/webapps/community/repository/view_workflow.mako @@ -20,9 +20,9 @@ can_rate = in_tool_shed and not is_new and trans.user and repository.user != trans.user can_view_change_log = in_tool_shed and not is_new if can_push: - browse_label = 'Browse or delete repository files' + browse_label = 'Browse or delete repository tip files' else: - browse_label = 'Browse repository files' + browse_label = 'Browse repository tip files' %><%! Repository URL: https://bitbucket.org/galaxy/galaxy-central/ -- This is a commit notification from bitbucket.org. You are receiving this because you have the service enabled, addressing the recipient of this email.
participants (1)
-
Bitbucket