Organize logging levels #3893
Organize logging levels #3893
Conversation
Log only what we need as ERROR when it's something that we need/want to take a look that something could be a bug/issue. There are some ERROR that were replaced by WARNING since they are some failures but that we don't need to take a look and the message shown to the user should be enough for them.
|
I'll take this one on however the CI issues look real. |
|
@davidfischer I didn't understand your comment and why you assigned this PR to you :P , but I just fixed the linting issues reported by travis. |
| @@ -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) | |||
There was a problem hiding this comment.
Can we start using single quotes ' around all the strings? 🙏
There was a problem hiding this comment.
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
readthedocs/projects/tasks.py
Outdated
| # Catch unhandled errors when syncing | ||
| except RepositoryError as e: | ||
| # Do not log as ERROR handled exceptions | ||
| log.warning('There was an error with the repository: msg=%s', e.msg) |
There was a problem hiding this comment.
We could pass exc_info=True to log.warning if you want the full exception.
|
In theory after this PR get deployed, we should be seeing only errors that we should take a look and do not ignore in Sentry. There could be a little more tweak to do maybe, but the general problem should be fixed. |
Log only what we need as ERROR when it's something that we need/want to take a look that something could be a bug/issue.
There are some ERROR that were replaced by WARNING since they are some failures but that we don't need to take a look and the message shown to the user should be enough for them.
Check #3898 for style changes