-
Notifications
You must be signed in to change notification settings - Fork 6
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
Logging setup #8
Comments
Thanks a lot! |
Thanks for the quick fix 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Hi there,
thank you for your awesome work here! I just have one tiny problem with your module:
Importing your module breaks the logging setup of the importing module because you're calling basicConfig (which cannot be used twice) on import here
python-fhem/fhem/fhem/__init__.py
Line 36 in f466959
I think it would be good practice to remove that call entirely so that the user of the module can decide how he want's to setup his logger. If you insist on setting the logLevel I would suggest using
logger.setLevel
which only modifies your own logger.An alternative would be calling basicConfig in your class initialization, in case the user hasn't done it so far. This ensures that a logger is properly setup but doesn't overwrite anything if the user has already done his own logging setup (which usually happens after importing modules but before initializing any classes).
But as I said, I would leave the choices of logging setup to the user. Everyone has different loggging needs ;)
The text was updated successfully, but these errors were encountered: