From 949dcfdbc34331de7f0aa321e67e1255a7627306 Mon Sep 17 00:00:00 2001 From: Nicola Soranzo Date: Wed, 24 Oct 2018 12:54:33 +0100 Subject: [PATCH] Do not log exceptions twice `logging.exception()` already adds the the exception information after the supplied message. Also: the `message` attribute of `Exception` class has been dropped in Python 3, always use `str(e)`. xref. https://github.com/galaxyproject/galaxy/issues/1715 --- lib/galaxy/authnz/managers.py | 26 +++++++++---------- lib/galaxy/jobs/runners/__init__.py | 2 +- lib/galaxy/jobs/runners/drmaa.py | 8 +++--- lib/galaxy/managers/cloud.py | 2 +- .../migrate/versions/0141_add_oidc_tables.py | 8 +++--- .../0142_change_numeric_metric_precision.py | 4 +-- .../versions/0143_add_cloudauthz_tables.py | 8 +++--- lib/galaxy/objectstore/cloud.py | 8 +++--- lib/galaxy/tools/__init__.py | 4 +-- lib/galaxy/util/pastescript/serve.py | 2 +- lib/galaxy/webapps/galaxy/api/cloudauthz.py | 5 ++-- .../galaxy/controllers/admin_toolshed.py | 2 +- .../webapps/galaxy/controllers/dataset.py | 8 +++--- .../tools/tool_panel_manager.py | 4 +-- lib/tool_shed/util/readme_util.py | 11 ++++---- .../api/reset_metadata_on_repositories.py | 8 +++--- test/base/populators.py | 4 +-- tools/data_source/upload.py | 2 +- 18 files changed, 56 insertions(+), 60 deletions(-) diff --git a/lib/galaxy/authnz/managers.py b/lib/galaxy/authnz/managers.py index a91cc933620..4b1a820b0c6 100644 --- a/lib/galaxy/authnz/managers.py +++ b/lib/galaxy/authnz/managers.py @@ -65,7 +65,7 @@ class AuthnzManager(object): except ImportError: raise except ParseError as e: - raise ParseError("Invalid configuration at `{}`: {} -- unable to continue.".format(config_file, e.message)) + raise ParseError("Invalid configuration at `{}`: {} -- unable to continue.".format(config_file, e)) def _parse_oidc_backends_config(self, config_file): self.oidc_backends_config = {} @@ -91,9 +91,7 @@ class AuthnzManager(object): except ImportError: raise except ParseError as e: - raise ParseError("Invalid configuration at `{}`: {} -- unable to continue.".format(config_file, e.message)) - # except Exception as e: - # raise Exception("Malformed OIDC Configuration XML -- unable to continue. {}".format(e.message)) + raise ParseError("Invalid configuration at `{}`: {} -- unable to continue.".format(config_file, e)) def _parse_google_config(self, config_xml): rtv = { @@ -117,7 +115,7 @@ class AuthnzManager(object): try: return True, "", PSAAuthnz(provider, self.oidc_config, self.oidc_backends_config[provider]) except Exception as e: - log.exception('An error occurred when loading PSAAuthnz: ', str(e)) + log.exception('An error occurred when loading PSAAuthnz') return False, str(e), None else: msg = 'The requested identity provider, `{}`, is not a recognized/expected provider'.format(provider) @@ -200,8 +198,8 @@ class AuthnzManager(object): if success is False: return False, message, None return True, "Redirecting to the `{}` identity provider for authentication".format(provider), backend.authenticate(trans) - except Exception as e: - msg = 'An error occurred when authenticating a user on `{}` identity provider: {}'.format(provider, str(e)) + except Exception: + msg = 'An error occurred when authenticating a user on `{}` identity provider'.format(provider) log.exception(msg) return False, msg, None @@ -211,8 +209,8 @@ class AuthnzManager(object): if success is False: return False, message, (None, None) return True, message, backend.callback(state_token, authz_code, trans, login_redirect_url) - except Exception as e: - msg = 'An error occurred when handling callback from `{}` identity provider; {}'.format(provider, str(e)) + except Exception: + msg = 'An error occurred when handling callback from `{}` identity provider'.format(provider) log.exception(msg) return False, msg, (None, None) @@ -222,9 +220,9 @@ class AuthnzManager(object): if success is False: return False, message, None return backend.disconnect(provider, trans, disconnect_redirect_url) - except Exception as e: - msg = 'An error occurred when disconnecting authentication with `{}` identity provider for user `{}`; ' \ - '{}'.format(provider, trans.user.username, str(e)) + except Exception: + msg = 'An error occurred when disconnecting authentication with `{}` identity provider for user `{}`' \ + .format(provider, trans.user.username) log.exception(msg) return False, msg, None @@ -265,5 +263,5 @@ class AuthnzManager(object): ca = CloudAuthz() return ca.authorize(cloudauthz.provider, config) except CloudAuthzBaseException as e: - log.exception(e.message) - raise exceptions.AuthenticationFailed(e.message) + log.exception("Error while requesting temporary access credentials") + raise exceptions.AuthenticationFailed(str(e)) diff --git a/lib/galaxy/jobs/runners/__init__.py b/lib/galaxy/jobs/runners/__init__.py index 550025f984d..0ba8ec84114 100644 --- a/lib/galaxy/jobs/runners/__init__.py +++ b/lib/galaxy/jobs/runners/__init__.py @@ -193,7 +193,7 @@ class BaseJobRunner(object): ) except Exception as e: log.exception("(%s) Failure preparing job" % job_id) - job_wrapper.fail(e.message if hasattr(e, 'message') else "Job preparation failed", exception=True) + job_wrapper.fail(str(e), exception=True) return False if not job_wrapper.runner_command_line: diff --git a/lib/galaxy/jobs/runners/drmaa.py b/lib/galaxy/jobs/runners/drmaa.py index 77e23a0bb81..dddbef1b17a 100644 --- a/lib/galaxy/jobs/runners/drmaa.py +++ b/lib/galaxy/jobs/runners/drmaa.py @@ -282,9 +282,9 @@ class DRMAAJobRunner(AsynchronousJobRunner): log.warning("(%s/%s) unable to communicate with DRM: %s", galaxy_id_tag, external_job_id, e) new_watched.append(ajs) continue - except Exception as e: + except Exception: # so we don't kill the monitor thread - log.exception("(%s/%s) unable to check job status: %s" % (galaxy_id_tag, external_job_id, e)) + log.exception("(%s/%s) unable to check job status" % (galaxy_id_tag, external_job_id)) log.warning("(%s/%s) job will now be errored" % (galaxy_id_tag, external_job_id)) ajs.fail_message = "Cluster could not complete job" self.work_queue.put((self.fail_job, ajs)) @@ -329,8 +329,8 @@ class DRMAAJobRunner(AsynchronousJobRunner): log.info("(%s/%s) Removed from DRM queue at user's request" % (job.get_id(), ext_id)) except drmaa.InvalidJobException: log.exception("(%s/%s) User killed running job, but it was already dead" % (job.get_id(), ext_id)) - except Exception as e: - log.exception("(%s/%s) User killed running job, but error encountered removing from DRM queue: %s" % (job.get_id(), ext_id, e)) + except Exception: + log.exception("(%s/%s) User killed running job, but error encountered removing from DRM queue" % (job.get_id(), ext_id)) def recover(self, job, job_wrapper): """Recovers jobs stuck in the queued/running state when Galaxy started""" diff --git a/lib/galaxy/managers/cloud.py b/lib/galaxy/managers/cloud.py index 86515f6a2cd..bcfac6d5faf 100644 --- a/lib/galaxy/managers/cloud.py +++ b/lib/galaxy/managers/cloud.py @@ -316,6 +316,6 @@ class CloudManager(sharable.SharableModelManager): downloaded.append(object_label) except Exception as e: log.debug("Failed to download dataset to cloud, maybe invalid or unauthorized credentials. " - "{}".format(e.message)) + "{}".format(e)) failed.append(object_label) return downloaded, failed diff --git a/lib/galaxy/model/migrate/versions/0141_add_oidc_tables.py b/lib/galaxy/model/migrate/versions/0141_add_oidc_tables.py index 0f6a5cac3eb..cbbac1a3e9c 100644 --- a/lib/galaxy/model/migrate/versions/0141_add_oidc_tables.py +++ b/lib/galaxy/model/migrate/versions/0141_add_oidc_tables.py @@ -69,8 +69,8 @@ def upgrade(migrate_engine): psa_nonce.create() psa_partial.create() oidc_user_authnz_tokens.create() - except Exception as e: - log.exception("Creating OIDC table failed: %s" % str(e)) + except Exception: + log.exception("Creating OIDC table failed") def downgrade(migrate_engine): @@ -83,5 +83,5 @@ def downgrade(migrate_engine): psa_nonce.drop() psa_partial.drop() oidc_user_authnz_tokens.drop() - except Exception as e: - log.exception("Dropping OIDC table failed: %s" % str(e)) + except Exception: + log.exception("Dropping OIDC table failed") diff --git a/lib/galaxy/model/migrate/versions/0142_change_numeric_metric_precision.py b/lib/galaxy/model/migrate/versions/0142_change_numeric_metric_precision.py index 2156bcf2b20..b707547137b 100644 --- a/lib/galaxy/model/migrate/versions/0142_change_numeric_metric_precision.py +++ b/lib/galaxy/model/migrate/versions/0142_change_numeric_metric_precision.py @@ -21,8 +21,8 @@ def upgrade(migrate_engine): t.c.metric_value.alter(type=Numeric(26, 7)) t = Table("task_metric_numeric", metadata, autoload=True) t.c.metric_value.alter(type=Numeric(26, 7)) - except Exception as e: - log.exception("Modifying numeric column failed: %s", str(e)) + except Exception: + log.exception("Modifying numeric column failed") def downgrade(migrate_engine): diff --git a/lib/galaxy/model/migrate/versions/0143_add_cloudauthz_tables.py b/lib/galaxy/model/migrate/versions/0143_add_cloudauthz_tables.py index 6767e52b54b..18319b62c9b 100644 --- a/lib/galaxy/model/migrate/versions/0143_add_cloudauthz_tables.py +++ b/lib/galaxy/model/migrate/versions/0143_add_cloudauthz_tables.py @@ -32,8 +32,8 @@ def upgrade(migrate_engine): try: cloudauthz.create() - except Exception as e: - log.exception("Failed to create cloudauthz table: {}".format(str(e))) + except Exception: + log.exception("Failed to create cloudauthz table") def downgrade(migrate_engine): @@ -42,5 +42,5 @@ def downgrade(migrate_engine): try: cloudauthz.drop() - except Exception as e: - log.exception("Failed to drop cloudauthz table: {}".format(str(e))) + except Exception: + log.exception("Failed to drop cloudauthz table") diff --git a/lib/galaxy/objectstore/cloud.py b/lib/galaxy/objectstore/cloud.py index 8d1b3b43f8f..e185681c4a8 100644 --- a/lib/galaxy/objectstore/cloud.py +++ b/lib/galaxy/objectstore/cloud.py @@ -178,13 +178,13 @@ class Cloud(ObjectStore): bucket = self.conn.storage.buckets.create(bucket_name) log.debug("Using cloud ObjectStore with bucket '%s'", bucket.name) return bucket - except InvalidNameException as e: - log.exception("Invalid bucket name -- unable to continue. {}".format(e.message)) + except InvalidNameException: + log.exception("Invalid bucket name -- unable to continue") raise - except Exception as e: + except Exception: # These two generic exceptions will be replaced by specific exceptions # once proper exceptions are exposed by CloudBridge. - log.exception("Could not get bucket '{}'. {}".format(bucket_name, e.message)) + log.exception("Could not get bucket '{}'".format(bucket_name)) raise Exception def _fix_permissions(self, rel_path): diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index 6c2b580b800..bfbdfc812f5 100755 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -1968,9 +1968,9 @@ class Tool(Dictifiable): tool_dict['value'] = input.value_to_basic(state_inputs.get(input.name, initial_value), self.app, use_security=True) tool_dict['default_value'] = input.value_to_basic(initial_value, self.app, use_security=True) tool_dict['text_value'] = input.value_to_display_text(tool_dict['value']) - except Exception as e: + except Exception: tool_dict = input.to_dict(request_context) - log.exception('tools::to_json() - Skipping parameter expansion \'%s\': %s.' % (input.name, e)) + log.exception("tools::to_json() - Skipping parameter expansion '%s'", input.name) pass if input_index >= len(group_inputs): group_inputs.append(tool_dict) diff --git a/lib/galaxy/util/pastescript/serve.py b/lib/galaxy/util/pastescript/serve.py index 8bf8a3cbcce..7fb10f669ae 100644 --- a/lib/galaxy/util/pastescript/serve.py +++ b/lib/galaxy/util/pastescript/serve.py @@ -1060,6 +1060,6 @@ def invoke(command, command_name, options, args): runner = command(command_name) exit_code = runner.run(args) except BadCommand as e: - print(e.message) + print(e) exit_code = e.exit_code sys.exit(exit_code) diff --git a/lib/galaxy/webapps/galaxy/api/cloudauthz.py b/lib/galaxy/webapps/galaxy/api/cloudauthz.py index 0c7a79e7eff..13fe5350be6 100644 --- a/lib/galaxy/webapps/galaxy/api/cloudauthz.py +++ b/lib/galaxy/webapps/galaxy/api/cloudauthz.py @@ -123,7 +123,7 @@ class CloudAuthzController(BaseAPIController): raise e # No two authorization configuration with - # exact same key/value should not exist. + # exact same key/value should exist. for ca in trans.user.cloudauthzs: if ca.equals(trans.user.id, provider, authn_id, config): log.debug("Rejected user `{}`'s request to create cloud authorization because a similar config " @@ -142,8 +142,7 @@ class CloudAuthzController(BaseAPIController): log.debug('Created a new cloudauthz record for the user id `{}` '.format(str(trans.user.id))) trans.response.status = '200' return view - except Exception as e: - log.exception(msg_template.format(e.message)) + log.exception(msg_template.format("exception while creating the new cloudauthz record")) raise InternalServerError('An unexpected error has occurred while responding to the create request of the ' 'cloudauthz API.' + str(e)) diff --git a/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py b/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py index 18ec840efbf..c26d27d15f3 100644 --- a/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py +++ b/lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py @@ -577,7 +577,7 @@ class AdminToolshed(AdminGalaxy): tsr_ids_for_monitoring = [trans.security.encode_id(tsr.id) for tsr in tool_shed_repositories] return json.dumps(tsr_ids_for_monitoring) except install_manager.RepositoriesInstalledException as e: - return self.message_exception(trans, e.message) + return self.message_exception(trans, str(e)) @web.expose @web.require_admin diff --git a/lib/galaxy/webapps/galaxy/controllers/dataset.py b/lib/galaxy/webapps/galaxy/controllers/dataset.py index 76425af96c3..9085b614f2c 100644 --- a/lib/galaxy/webapps/galaxy/controllers/dataset.py +++ b/lib/galaxy/webapps/galaxy/controllers/dataset.py @@ -872,9 +872,9 @@ class DatasetInterface(BaseUIController, UsesAnnotations, UsesItemRatings, UsesE trans.log_event("Dataset id %s marked as deleted" % str(id)) self.hda_manager.stop_creating_job(hda) trans.sa_session.flush() - except Exception as e: + except Exception: msg = 'HDA deletion failed (encoded: %s, decoded: %s)' % (dataset_id, id) - log.exception(msg + ': ' + str(e)) + log.exception(msg) trans.log_event(msg) message = 'Dataset deletion failed' status = 'error' @@ -966,8 +966,8 @@ class DatasetInterface(BaseUIController, UsesAnnotations, UsesItemRatings, UsesE except Exception: log.exception('Unable to purge dataset (%s) on purge of HDA (%s):' % (hda.dataset.id, hda.id)) trans.sa_session.flush() - except Exception as exc: - msg = 'HDA purge failed (encoded: %s, decoded: %s): %s' % (dataset_id, id, exc) + except Exception: + msg = 'HDA purge failed (encoded: %s, decoded: %s)' % (dataset_id, id) log.exception(msg) trans.log_event(msg) message = 'Dataset removal from disk failed' diff --git a/lib/tool_shed/galaxy_install/tools/tool_panel_manager.py b/lib/tool_shed/galaxy_install/tools/tool_panel_manager.py index 47292749a29..2f99c2c0098 100644 --- a/lib/tool_shed/galaxy_install/tools/tool_panel_manager.py +++ b/lib/tool_shed/galaxy_install/tools/tool_panel_manager.py @@ -86,8 +86,8 @@ class ToolPanelManager(object): root.append(elem) with RenamedTemporaryFile(config_filename, mode='w') as fh: fh.write(xml_to_string(root, pretty=True)) - except Exception as e: - log.exception("Exception in ToolPanelManager.config_elems_to_xml_file: \n %s", str(e)) + except Exception: + log.exception("Exception in ToolPanelManager.config_elems_to_xml_file") def generate_tool_elem(self, tool_shed, repository_name, changeset_revision, owner, tool_file_path, tool, tool_section): diff --git a/lib/tool_shed/util/readme_util.py b/lib/tool_shed/util/readme_util.py index 06c00968992..bf22f22a8be 100644 --- a/lib/tool_shed/util/readme_util.py +++ b/lib/tool_shed/util/readme_util.py @@ -43,8 +43,8 @@ def build_readme_files_dict(app, repository, changeset_revision, metadata, tool_ f = open(full_path_to_readme_file, 'r') text = unicodify(f.read()) f.close() - except Exception as e: - log.exception("Error reading README file '%s' from disk", str(relative_path_to_readme_file)) + except Exception: + log.exception("Error reading README file '%s' from disk", relative_path_to_readme_file) text = None if text: text_of_reasonable_length = basic_util.size_string(text) @@ -56,7 +56,7 @@ def build_readme_files_dict(app, repository, changeset_revision, metadata, tool_ text_of_reasonable_length = suc.set_image_paths(app, app.security.encode_id(repository.id), text_of_reasonable_length) - except Exception as e: + except Exception: log.exception("Exception in build_readme_files_dict, so images may not be properly displayed") finally: lock.release() @@ -81,9 +81,8 @@ def build_readme_files_dict(app, repository, changeset_revision, metadata, tool_ try: text = unicodify(fctx.data()) readme_files_dict[readme_file_name] = basic_util.size_string(text) - except Exception as e: - log.exception("Error reading README file '%s' from repository manifest: %s" % - (str(relative_path_to_readme_file), str(e))) + except Exception: + log.exception("Error reading README file '%s' from repository manifest", relative_path_to_readme_file) return readme_files_dict diff --git a/scripts/tool_shed/api/reset_metadata_on_repositories.py b/scripts/tool_shed/api/reset_metadata_on_repositories.py index b2791686c51..abd64e11f05 100644 --- a/scripts/tool_shed/api/reset_metadata_on_repositories.py +++ b/scripts/tool_shed/api/reset_metadata_on_repositories.py @@ -66,8 +66,8 @@ def main(options): url = '%s/api/repositories/reset_metadata_on_repository' % base_tool_shed_url try: submit(url, data, options.api) - except Exception as e: - log.exception(">>>>>>>>>>>>>>>Blew up on data: %s, exception: %s" % (str(data), str(e))) + except Exception: + log.exception(">>>>>>>>>>>>>>>Blew up on data: %s", data) # An nginx timeout undoubtedly occurred. sys.exit(1) else: @@ -76,8 +76,8 @@ def main(options): url = '%s/api/repositories/reset_metadata_on_repositories' % base_tool_shed_url try: submit(url, data, options.api) - except Exception as e: - log.exception(str(e)) + except Exception: + log.exception(">>>>>>>>>>>>>>>Blew up on data: %s", data) # An nginx timeout undoubtedly occurred. sys.exit(1) diff --git a/test/base/populators.py b/test/base/populators.py index fdb30428db8..a465fbd5025 100644 --- a/test/base/populators.py +++ b/test/base/populators.py @@ -223,7 +223,7 @@ class BaseDatasetPopulator(object): wait_on(has_active_jobs, "active jobs", timeout=timeout) except TimeoutAssertionError as e: jobs = self.history_jobs(history_id) - message = "Failed waiting on active jobs to complete, current jobs are [%s]. %s" % (jobs, e.message) + message = "Failed waiting on active jobs to complete, current jobs are [%s]. %s" % (jobs, e) raise TimeoutAssertionError(message) if assert_ok: @@ -1152,7 +1152,7 @@ def wait_on_state(state_func, desc="state", skip_states=["running", "queued", "n return wait_on(get_state, desc=desc, timeout=timeout) except TimeoutAssertionError as e: response = state_func() - raise TimeoutAssertionError("%s Current response containing state [%s]." % (str(e), response.json())) + raise TimeoutAssertionError("%s Current response containing state [%s]." % (e, response.json())) class GiPostGetMixin(object): diff --git a/tools/data_source/upload.py b/tools/data_source/upload.py index 920025180b0..d3dce6dfec2 100644 --- a/tools/data_source/upload.py +++ b/tools/data_source/upload.py @@ -283,7 +283,7 @@ def __main__(): else: metadata.append(add_file(dataset, registry, output_path)) except UploadProblemException as e: - metadata.append(file_err(e.message, dataset)) + metadata.append(file_err(str(e), dataset)) __write_job_metadata(metadata)