Skip to content

Commit

Permalink
Do not log exceptions twice
Browse files Browse the repository at this point in the history
`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. #1715
  • Loading branch information
nsoranzo committed Oct 24, 2018
1 parent 51ca8dc commit 949dcfd
Show file tree
Hide file tree
Showing 18 changed files with 56 additions and 60 deletions.
26 changes: 12 additions & 14 deletions lib/galaxy/authnz/managers.py
Expand Up @@ -65,7 +65,7 @@ def _parse_oidc_config(self, config_file):
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 = {}
Expand All @@ -91,9 +91,7 @@ def _parse_oidc_backends_config(self, config_file):
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 = {
Expand All @@ -117,7 +115,7 @@ def _get_authnz_backend(self, provider):
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)
Expand Down Expand Up @@ -200,8 +198,8 @@ def authenticate(self, provider, trans):
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

Expand All @@ -211,8 +209,8 @@ def callback(self, provider, state_token, authz_code, trans, login_redirect_url)
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)

Expand All @@ -222,9 +220,9 @@ def disconnect(self, provider, trans, disconnect_redirect_url=None):
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

Expand Down Expand Up @@ -265,5 +263,5 @@ def get_cloud_access_credentials(self, cloudauthz, sa_session, user_id, request=
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))
2 changes: 1 addition & 1 deletion lib/galaxy/jobs/runners/__init__.py
Expand Up @@ -193,7 +193,7 @@ def prepare_job(self, job_wrapper, include_metadata=False, include_work_dir_outp
)
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:
Expand Down
8 changes: 4 additions & 4 deletions lib/galaxy/jobs/runners/drmaa.py
Expand Up @@ -282,9 +282,9 @@ def check_watched_items(self):
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))
Expand Down Expand Up @@ -329,8 +329,8 @@ def stop_job(self, job):
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"""
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/managers/cloud.py
Expand Up @@ -316,6 +316,6 @@ def download(self, trans, history_id, provider, bucket_name, credentials, datase
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
8 changes: 4 additions & 4 deletions lib/galaxy/model/migrate/versions/0141_add_oidc_tables.py
Expand Up @@ -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):
Expand All @@ -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")
Expand Up @@ -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):
Expand Down
Expand Up @@ -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):
Expand All @@ -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")
8 changes: 4 additions & 4 deletions lib/galaxy/objectstore/cloud.py
Expand Up @@ -178,13 +178,13 @@ def _get_bucket(self, bucket_name):
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):
Expand Down
4 changes: 2 additions & 2 deletions lib/galaxy/tools/__init__.py
Expand Up @@ -1968,9 +1968,9 @@ def populate_model(self, request_context, inputs, state_inputs, group_inputs, ot
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)
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/util/pastescript/serve.py
Expand Up @@ -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)
5 changes: 2 additions & 3 deletions lib/galaxy/webapps/galaxy/api/cloudauthz.py
Expand Up @@ -123,7 +123,7 @@ def create(self, trans, payload, **kwargs):
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 "
Expand All @@ -142,8 +142,7 @@ def create(self, trans, payload, **kwargs):
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))
2 changes: 1 addition & 1 deletion lib/galaxy/webapps/galaxy/controllers/admin_toolshed.py
Expand Up @@ -577,7 +577,7 @@ def install_repositories(self, trans, **kwd):
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
Expand Down
8 changes: 4 additions & 4 deletions lib/galaxy/webapps/galaxy/controllers/dataset.py
Expand Up @@ -872,9 +872,9 @@ def _delete(self, trans, dataset_id):
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'
Expand Down Expand Up @@ -966,8 +966,8 @@ def _purge(self, trans, dataset_id):
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'
Expand Down
4 changes: 2 additions & 2 deletions lib/tool_shed/galaxy_install/tools/tool_panel_manager.py
Expand Up @@ -86,8 +86,8 @@ def config_elems_to_xml_file(self, config_elems, config_filename, tool_path):
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):
Expand Down
11 changes: 5 additions & 6 deletions lib/tool_shed/util/readme_util.py
Expand Up @@ -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)
Expand All @@ -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()
Expand All @@ -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


Expand Down
8 changes: 4 additions & 4 deletions scripts/tool_shed/api/reset_metadata_on_repositories.py
Expand Up @@ -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:
Expand All @@ -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)

Expand Down
4 changes: 2 additions & 2 deletions test/base/populators.py
Expand Up @@ -223,7 +223,7 @@ def has_active_jobs():
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:
Expand Down Expand Up @@ -1152,7 +1152,7 @@ def get_state():
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):
Expand Down
2 changes: 1 addition & 1 deletion tools/data_source/upload.py
Expand Up @@ -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)


Expand Down

0 comments on commit 949dcfd

Please sign in to comment.