Conversation
SebastiaanZ
approved these changes
Feb 20, 2020
jb3
approved these changes
Feb 20, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR is to refactor the logging prep in our
__init__.pymodule to avoid issues encountered with import sensitivity and overwritten logging settings.It simplifies some existing code and removed anything that was no longer useful, such as the
logmaticdependency and theaio_pikalog setup..The formatting of the logger was tweaked.
flake8-annotationshave also been updated 2.0 and adjusted to use theANNrule prefix.Background
Logging Levels and Handlers
Previously, we had a for loop that indiscriminately forced all registered individual loggers to be a set level and to each have a set list of handlers.
This resulted in us being forced to never import a library/module that needs to have logging explicitly set a certain way before this for loop lest it be overwritten with the generic settings.
This results in unexpected and undesirable behaviours, such as if we need to patch discord.py before anything uses it, it'll be forced back into DEBUG level logging by the time we run the bot, outputting excessive discord api logs.
Logmatic
The logmatic dependency was used exclusively for it's JSONFormatter, which is not being used for anything and isn't more human readable than standard log output.
Flake8 Annotations
CI checks failed to install
typed_astwhenflake8-annotationswas updated to v1.3:Might as well update and adjust to the new major release due to this small inconvenience.
Changes
The logmatic dependency was removed, the pipfile relocked and the log handlers kept to just the stream handler and the standard file handler.
The for loop for dynamic forcing of all existing logs to use set levels and handlers was removed in favour of a clear and explicit setup of known loggers.
The
aio_pikalogger setup was removed as it's no longer a dependency in this project.Formatting of the logs has been adjusted to:
v2.0 of
flake8-annotationswas put into the pipfile and updated in the lock, and thetox.inifile was updated to useANNprefix for the rules after the change fromTYPin previous versions.