Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Organize logging levels #3893

Merged
merged 3 commits into from Apr 5, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion readthedocs/builds/models.py
Expand Up @@ -247,7 +247,7 @@ def clean_build_path(self):
try:
path = self.get_build_path()
if path is not None:
log.debug('Removing build path {0} for {1}'.format(path, self))
log.debug('Removing build path %s for %s', path, self)
rmtree(path)
except OSError:
log.exception('Build path cleanup failed')
Expand Down
18 changes: 9 additions & 9 deletions readthedocs/builds/syncers.py
Expand Up @@ -58,8 +58,7 @@ def copy(cls, path, target, is_file=False, **__):
mkdir_cmd = ("ssh %s@%s mkdir -p %s" % (sync_user, server, target))
ret = os.system(mkdir_cmd)
if ret != 0:
log.info("COPY ERROR to app servers:")
log.info(mkdir_cmd)
log.error("Copy error to app servers: cmd=%s", mkdir_cmd)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we start using single quotes ' around all the strings? 🙏

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't do this manually. We are trying to use pre-commit more often that does this automatically.

The result of pre-commit ran under all the modified files by this PR is at: #3898

if is_file:
slash = ""
else:
Expand All @@ -75,8 +74,7 @@ def copy(cls, path, target, is_file=False, **__):
target=target))
ret = os.system(sync_cmd)
if ret != 0:
log.info("COPY ERROR to app servers.")
log.info(sync_cmd)
log.error("Copy error to app servers: cmd=%s", sync_cmd)


class DoubleRemotePuller(object):
Expand All @@ -100,8 +98,7 @@ def copy(cls, path, target, host, is_file=False, **__):
)
ret = os.system(mkdir_cmd)
if ret != 0:
log.info("MKDIR ERROR to app servers:")
log.info(mkdir_cmd)
log.error("MkDir error to app servers: cmd=%s", mkdir_cmd)
# Add a slash when copying directories
sync_cmd = (
"ssh {user}@{server} 'rsync -av "
Expand All @@ -114,8 +111,7 @@ def copy(cls, path, target, host, is_file=False, **__):
target=target))
ret = os.system(sync_cmd)
if ret != 0:
log.info("COPY ERROR to app servers.")
log.info(sync_cmd)
log.error("Copy error to app servers: cmd=%s", sync_cmd)


class RemotePuller(object):
Expand All @@ -142,7 +138,11 @@ def copy(cls, path, target, host, is_file=False, **__):
)
ret = os.system(sync_cmd)
if ret != 0:
log.error("COPY ERROR to app servers. Command: [{}] Return: [{}]".format(sync_cmd, ret))
log.error(
"Copy error to app servers. Command: [%s] Return: [%s]",
sync_cmd,
ret,
)


class Syncer(SettingsOverrideObject):
Expand Down
15 changes: 10 additions & 5 deletions readthedocs/core/management/commands/clean_builds.py
Expand Up @@ -44,14 +44,19 @@ def handle(self, *args, **options):
version = Version.objects.get(id=build['version'])
latest_build = version.builds.latest('date')
if latest_build.date > max_date:
log.warning('{0} is newer than {1}'.format(
latest_build, max_date))
log.warning(
'%s is newer than %s',
latest_build,
max_date,
)
path = version.get_build_path()
if path is not None:
log.info(
('Found stale build path for {0} '
'at {1}, last used on {2}').format(
version, path, latest_build.date))
'Found stale build path for %s at %s, last used on %s',
version,
path,
latest_build.date,
)
if not options['dryrun']:
version.clean_build_path()
except Version.DoesNotExist:
Expand Down
Expand Up @@ -53,4 +53,4 @@ def handle(self, *args, **options):
update_search(version.pk, commit,
delete_non_commit_files=False)
except Exception:
log.exception('Reindex failed for {}'.format(version))
log.exception('Reindex failed for %s', version)
7 changes: 3 additions & 4 deletions readthedocs/core/signals.py
Expand Up @@ -57,10 +57,9 @@ def decide_if_cors(sender, request, **kwargs): # pylint: disable=unused-argumen
project = Project.objects.get(slug=project_slug)
except Project.DoesNotExist:
log.warning(
'Invalid project passed to domain. [{project}:{domain}'.format(
project=project_slug,
domain=host,
)
'Invalid project passed to domain. [%s:%s]',
project_slug,
host,
)
return False

Expand Down
11 changes: 6 additions & 5 deletions readthedocs/core/views/hooks.py
Expand Up @@ -233,12 +233,13 @@ def gitlab_build(request): # noqa: D205
log.info(
'GitLab webhook search: url=%s branches=%s',
search_url,
branches
branches,
)
projects = get_project_from_url(search_url)
if projects:
return _build_url(search_url, projects, branches)
log.error('Project match not found: url=%s', search_url)

log.info('Project match not found: url=%s', search_url)
return HttpResponseNotFound('Project match not found')
return HttpResponse('Method not allowed, POST is required', status=405)

Expand Down Expand Up @@ -294,7 +295,7 @@ def bitbucket_build(request):
log.info(
'Bitbucket webhook search: url=%s branches=%s',
search_url,
branches
branches,
)
log.debug('Bitbucket webhook payload:\n\n%s\n\n', data)
projects = get_project_from_url(search_url)
Expand All @@ -304,10 +305,10 @@ def bitbucket_build(request):
log.error(
'Commit/branch not found url=%s branches=%s',
search_url,
branches
branches,
)
return HttpResponseNotFound('Commit/branch not found')
log.error('Project match not found: url=%s', search_url)
log.info('Project match not found: url=%s', search_url)
return HttpResponseNotFound('Project match not found')
return HttpResponse('Method not allowed, POST is required', status=405)

Expand Down
15 changes: 7 additions & 8 deletions readthedocs/doc_builder/environments.py
Expand Up @@ -445,12 +445,12 @@ def handle_exception(self, exc_type, exc_value, _):
a failure and the context will be gracefully exited.
"""
if exc_type is not None:
log.error(LOG_TEMPLATE
.format(project=self.project.slug,
version=self.version.slug,
msg=exc_value),
exc_info=True)
if not issubclass(exc_type, BuildEnvironmentWarning):
log.error(LOG_TEMPLATE
.format(project=self.project.slug,
version=self.version.slug,
msg=exc_value),
exc_info=True)
self.failure = exc_value
return True

Expand Down Expand Up @@ -574,10 +574,9 @@ def update_build(self, state=None):
try:
api_v2.build(self.build['id']).put(self.build)
except HttpClientError as e:
log.error(
"Unable to update build: id=%d error=%s",
log.exception(
"Unable to update build: id=%d",
self.build['id'],
e.content,
)
except Exception:
log.exception("Unknown build exception")
Expand Down
7 changes: 5 additions & 2 deletions readthedocs/oauth/services/base.py
Expand Up @@ -138,7 +138,7 @@ def paginate(self, url, **kwargs):
return results
# Catch specific exception related to OAuth
except InvalidClientIdError:
log.error('access_token or refresh_token failed: %s', url)
log.warning('access_token or refresh_token failed: %s', url)
raise Exception('You should reconnect your account')
# Catch exceptions with request or deserializing JSON
except (RequestException, ValueError):
Expand All @@ -149,7 +149,10 @@ def paginate(self, url, **kwargs):
except ValueError:
debug_data = resp.content
log.debug(
'paginate failed at %s with response: %s', url, debug_data)
'Paginate failed at %s with response: %s',
url,
debug_data,
)
else:
return []

Expand Down
53 changes: 34 additions & 19 deletions readthedocs/oauth/services/bitbucket.py
Expand Up @@ -45,8 +45,7 @@ def sync_repositories(self):
for repo in repos:
self.create_repository(repo)
except (TypeError, ValueError) as e:
log.error('Error syncing Bitbucket repositories: %s',
str(e), exc_info=True)
log.exception('Error syncing Bitbucket repositories')
raise Exception('Could not sync your Bitbucket repositories, '
'try reconnecting your account')

Expand Down Expand Up @@ -80,8 +79,7 @@ def sync_teams(self):
for repo in repos:
self.create_repository(repo, organization=org)
except ValueError as e:
log.error('Error syncing Bitbucket organizations: %s',
str(e), exc_info=True)
log.exception('Error syncing Bitbucket organizations')
raise Exception('Could not sync your Bitbucket team repositories, '
'try reconnecting your account')

Expand Down Expand Up @@ -220,19 +218,27 @@ def setup_webhook(self, project):
recv_data = resp.json()
integration.provider_data = recv_data
integration.save()
log.info('Bitbucket webhook creation successful for project: %s',
project)
log.info(
'Bitbucket webhook creation successful for project: %s',
project,
)
return (True, resp)
# Catch exceptions with request or deserializing JSON
except (RequestException, ValueError):
log.error('Bitbucket webhook creation failed for project: %s',
project, exc_info=True)
log.exception(
'Bitbucket webhook creation failed for project: %s',
project,
)
else:
log.error('Bitbucket webhook creation failed for project: %s',
project)
log.exception(
'Bitbucket webhook creation failed for project: %s',
project,
)
try:
log.debug('Bitbucket webhook creation failure response: %s',
resp.json())
log.debug(
'Bitbucket webhook creation failure response: %s',
resp.json(),
)
except ValueError:
pass
return (False, resp)
Expand Down Expand Up @@ -263,20 +269,29 @@ def update_webhook(self, project, integration):
recv_data = resp.json()
integration.provider_data = recv_data
integration.save()
log.info('Bitbucket webhook update successful for project: %s',
project)
log.info(
'Bitbucket webhook update successful for project: %s',
project,
)
return (True, resp)
# Catch exceptions with request or deserializing JSON
except (KeyError, RequestException, ValueError):
log.error('Bitbucket webhook update failed for project: %s',
project, exc_info=True)
log.exception(
'Bitbucket webhook update failed for project: %s',
project,
)
else:
log.error('Bitbucket webhook update failed for project: %s',
project)
log.exception(
'Bitbucket webhook update failed for project: %s',
project,
)
# Response data should always be JSON, still try to log if not though
try:
debug_data = resp.json()
except ValueError:
debug_data = resp.content
log.debug('Bitbucket webhook update failure response: %s', debug_data)
log.debug(
'Bitbucket webhook update failure response: %s',
debug_data,
)
return (False, resp)
51 changes: 32 additions & 19 deletions readthedocs/oauth/services/github.py
Expand Up @@ -42,8 +42,7 @@ def sync_repositories(self):
for repo in repos:
self.create_repository(repo)
except (TypeError, ValueError) as e:
log.error('Error syncing GitHub repositories: %s',
str(e), exc_info=True)
log.exception('Error syncing GitHub repositories')
raise Exception('Could not sync your GitHub repositories, '
'try reconnecting your account')

Expand All @@ -62,8 +61,7 @@ def sync_organizations(self):
for repo in org_repos:
self.create_repository(repo, organization=org_obj)
except (TypeError, ValueError) as e:
log.error('Error syncing GitHub organizations: %s',
str(e), exc_info=True)
log.exception('Error syncing GitHub organizations')
raise Exception('Could not sync your GitHub organizations, '
'try reconnecting your account')

Expand Down Expand Up @@ -211,18 +209,25 @@ def setup_webhook(self, project):
return (True, resp)
# Catch exceptions with request or deserializing JSON
except (RequestException, ValueError):
log.error('GitHub webhook creation failed for project: %s',
project, exc_info=True)
log.exception(
'GitHub webhook creation failed for project: %s',
project,
)
else:
log.error('GitHub webhook creation failed for project: %s',
project)
# Response data should always be JSON, still try to log if not though
log.exception(
'GitHub webhook creation failed for project: %s',
project,
)
# Response data should always be JSON, still try to log if not
# though
try:
debug_data = resp.json()
except ValueError:
debug_data = resp.content
log.debug('GitHub webhook creation failure response: %s',
debug_data)
log.debug(
'GitHub webhook creation failure response: %s',
debug_data,
)
return (False, resp)

def update_webhook(self, project, integration):
Expand Down Expand Up @@ -251,22 +256,30 @@ def update_webhook(self, project, integration):
recv_data = resp.json()
integration.provider_data = recv_data
integration.save()
log.info('GitHub webhook creation successful for project: %s',
project)
log.info(
'GitHub webhook creation successful for project: %s',
project,
)
return (True, resp)
# Catch exceptions with request or deserializing JSON
except (RequestException, ValueError):
log.error('GitHub webhook update failed for project: %s',
project, exc_info=True)
log.exception(
'GitHub webhook update failed for project: %s',
project,
)
else:
log.error('GitHub webhook update failed for project: %s',
project)
log.exception(
'GitHub webhook update failed for project: %s',
project,
)
try:
debug_data = resp.json()
except ValueError:
debug_data = resp.content
log.debug('GitHub webhook creation failure response: %s',
debug_data)
log.debug(
'GitHub webhook creation failure response: %s',
debug_data,
)
return (False, resp)

@classmethod
Expand Down
5 changes: 4 additions & 1 deletion readthedocs/oauth/services/gitlab.py
Expand Up @@ -336,7 +336,10 @@ def update_webhook(self, project, integration):
log.exception(
'GitLab webhook update failed for project: %s', project)
else:
log.error('GitLab webhook update failed for project: %s', project)
log.exception(
'GitLab webhook update failed for project: %s',
project,
)
try:
debug_data = resp.json()
except ValueError:
Expand Down