feat: use module names for loggers #136
Conversation
|
|
padamstx
changed the title
|
@datawookie Andrew, thank you for the PR! We'll get this reviewed and let you know about any feedback, etc. @rmkeezer @pyrooka Hey guys, this seems to be a pretty straightforward PR. Can you give it a look to make sure I didn't miss anything? :) |
|
@datawookie Andrew, one question... given these changes, can you briefly describe how one would turn on or off the logging for a specific logger/module? Thanks |
There was a problem hiding this comment.
Hey @datawookie! The PR looks good overall, I've just requested some code style related changes. I don't want to be too picky, just trying to follow the conventions.
ibm_cloud_sdk_core/token_managers/vpc_instance_token_manager.py
Outdated
Show resolved
Hide resolved
|
Hi @pyrooka, I'm all about conventions, so very happy to apply those changes. Thank you! |
|
@datawookie With your latest changes, the build is now failing during the unti test execution due to some sort of indentation issue. Are you able to build/test cleanly in your local sandbox? (this is the same command that is run during the Travis build) Edit: I recommend that you use a virtualenv to avoid polluting your machine with dependencies from this project :) |
ibm_cloud_sdk_core/base_service.py
Outdated
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| #pylint: disable=too-many-instance-attributes | ||
| #pylint: disable=too-many-locals | ||
| class BaseService: | ||
|
|
||
|
|
||
| #pylint: disable=too-many-instance-attributes | ||
| #pylint: disable=too-many-locals | ||
| class BaseService: |
There was a problem hiding this comment.
@datawookie I should have said that my suggestion was invalid in that context, because I wasn't able to select the whole code block.
You can easily fix the duplicated code manually, then check it as @padamstx mentioned.
|
Hi @pyrooka, ah, crap. No problem. I applied the changes locally and then force pushed. Also changed my commit messages to use conventional commits. Hope that this is all good now. Thanks, Andrew. |
There was a problem hiding this comment.
@datawookie I approve it to avoid the re-review, but there is one more thing to fix. Sorry, if I was confusing!
|
Hi @pyrooka, I've moved the code. Thanks, Andrew. |
|
@datawookie I think all the code changes look good now, but with your last commit it looks like you added the "nohup.out" file inadvertently. Please remove that then update your branch and we can get this PR merged in. Thanks! |
|
@pyrooka fixed! |
# [3.14.0](v3.13.2...v3.14.0) (2022-01-14) ### Features * use module names for loggers ([#136](#136)) ([36523c8](36523c8))
|
🎉 This PR is included in version 3.14.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Hi!
We've been using this package for around a year now and have really enjoyed how simple it makes submitting queries to the Watson API.
However, we'd like to be able to control the level of logging produced by the library. At present this is difficult since all of the modules what use logging do it via the root logger (so it's impossible to pin-point just what we are turning off).
In this MR I have used the module name for each module that employs logging. This means that clients using the package can now selecting turn logging on or off for this package. This is the behaviour that is implemented in many popular Python packages like
requests.Thanks, Andrew.