-
Notifications
You must be signed in to change notification settings - Fork 664
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
Refactor logging #3000
Refactor logging #3000
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see a regression: "Found config file" which is printed before does no longer happen with this patch.
b565ebd
to
f23bd81
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please fix the conflicts and ping me when this seams ready for review? (no known undesired side-effects). I will have to test it manually to assure we do not mess the logging even more with it but I need your confirmation first.
When we removed custom logging handlers, we left behind a few helper classes that we are not using anymore. This commit finishes the job of removing custom bits from the codebase.
The main purpose of changes in current commit is to centralize the logger configuration. Previously, molecule configured each and every logger, creating new handler for each one, which made little sense. In the new implementation, we only configure the root molecule logger. All children logger instances inherit the configuration, making away with duplication. With our logging refactor, it also became feasible to use Python's built-in logging facilities as instructed in the official docs. Do note that we left the get_logger function in the logger module because that piece of code is used by various molecule plugins (for example, drivers) to obtain the properly configured logger instance. But we again made sure we just nest the logger below the molecule root logger and let it inherit the configuration.
Up until now, we ignored the verbosity setting and only used it to control the verbosity of the Ansible subprocess. This commit reuses the verbosity flag to also control the molecule verbosity.
f23bd81
to
c200b9e
Compare
I rebased the PR and tested it again locally. The only visible change of behavior from the master branch now should be the suppression of debug messages when running Molecule with no verbosity or debug flags. |
Molecule reinvented the logging for some reason and did not use the standard Python logging facilities as they are meant to be used. This PR brings some sanity to the logging part of the code.
See commit messages for more detailed change descriptions.
PR Type
Fixes: #2999