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

Fixes logging by removing root logger and handlers from library #19

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

DavidBowley
Copy link

According to the Python Logging Documentation:

Note: It is strongly advised that you do not log to the root logger in your library. Instead, use a logger with a unique and easily identifiable name, such as the name for your library’s top-level package or module. Logging to the root logger will make it difficult or impossible for the application developer to configure the logging verbosity or handlers of your library as they wish.

and

Note: It is strongly advised that you do not add any handlers other than NullHandler to your library’s loggers. This is because the configuration of handlers is the prerogative of the application developer who uses your library. The application developer knows their target audience and what handlers are most appropriate for their application: if you add handlers ‘under the hood’, you might well interfere with their ability to carry out unit tests and deliver logs which suit their requirements.

The root logger accessed through logging.basicConfig() was removed along with the handler it automatically adds. This allows the application developer to have full control over the output/formatting of the logs. Without this, I had no control over the logs and ended up getting doubled logs sent to the console - this PR fixes that.

Also added the following:

  • Updated Settings page with some basic instructions on how to configure the root logger in the developer's application
  • Converted some of the print statements to Python 3 compatible syntax
  • Added a Python .gitignore file

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant