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

Configure basic top-level logger #212

Merged
merged 5 commits into from
Feb 25, 2020

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Feb 25, 2020

Fixes issue #:
Closes #210

Description of the changes being introduced by the pull request:
As pointed out by @FlorianVeaux in #210, securesystemslib does not correctly initialize its loggers. As a consequence the logging module logs a one-off "No handler could be found ..." warning when used for the first time (only on Python 2).
The logging module then calls basicConfig, which attaches a basic StreamHandler to the root logger, to which securesystemslib's log messages are propagated.

This PR adds the configuration of a simple securesystemslib top-level logger, with an attached StreamHandler and log level WARNING, to which all other securesystemslib loggers default.
This removes the one-off warning, while providing a similar logging behavior as before and clear instructions (see added comments) on how to customize (format, silence, etc...) securesystemslib's logging.

The PR further:

  • removes unused loggers
  • establishes __name__ logger naming convention
  • replaces a deprecated logging method

Please verify and check that the pull request fulfils the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

Replace hardcoded logger names with __name__. This also helps to
enforce a logger hierarchy, i.e. "securesystemslib" overrules
"securesystemslib.keys".
- remove obsolete comment (from when sslib was part of tuf)
- reword docstring to disambiguate printing and logging
As pointed out by @FlorianVeaux in secure-systems-lab#210, securesystemslib does not
correctly initialize its loggers. As a consequence the logging
module logs a one-off "No handler could be found ..." warning when
used for the first time (only on Python 2).

The logging module then calls basicConfig, which attaches a
basic StreamHandler to the root logger, to which securesystemslib's
log messages are propagated.

This commit adds the configuration of a simple securesystemslib
top-level logger, with an attached StreamHandler and log level
WARNING, to which all other securesystemslib loggers default.

This removes the one-off warning, while providing a similar logging
behavior as before and clear instructions (see added comments) on
how to customize (format, silence, etc...) securesystemslib's logging.
@lukpueh
Copy link
Member Author

lukpueh commented Feb 25, 2020

@trishankatdatadog, @FlorianVeaux, please let me know if this works for you.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 98.712% when pulling 7f06845 on lukpueh:fix-logging into 2f2dfa9 on secure-systems-lab:master.

Copy link
Collaborator

@joshuagl joshuagl left a comment

Choose a reason for hiding this comment

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

Great set of changes, thanks @lukpueh

Copy link
Contributor

@FlorianVeaux FlorianVeaux left a comment

Choose a reason for hiding this comment

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

Works great for me, thanks 🙇

@lukpueh lukpueh merged commit 459496b into secure-systems-lab:master Feb 25, 2020
@lukpueh lukpueh mentioned this pull request Feb 25, 2020
@lukpueh
Copy link
Member Author

lukpueh commented Feb 25, 2020

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.

Module is logging warnings on import
4 participants