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
This commit is contained in:
Nicola Soranzo
2018-10-24 12:57:05 +01:00
parent 51ca8dc0f5
commit 949dcfdbc3
18 changed files with 56 additions and 60 deletions
+12 -14
View File
@@ -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))
+1 -1
View File
@@ -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:
+4 -4
View File
@@ -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"""
+1 -1
View File
@@ -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
@@ -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")
@@ -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):
@@ -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")
+4 -4
View File
@@ -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):
+2 -2
View File
@@ -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)
+1 -1
View File
@@ -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)
+2 -3
View File
@@ -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))
@@ -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
@@ -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'
@@ -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):
+5 -6
View File
@@ -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
@@ -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)
+2 -2
View File
@@ -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):
+1 -1
View File
@@ -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)