pconf
pconf copied to clipboard
disable default logging
Fixes #34
Proposed Changes
This will suppress the warning if the user doesn't have a logging facility set up, which I don't think is the proper default behavior.
I'd rather support a solution that would either: a) Allow the logging of these warning to be configured externally b) If there is a logger set up, use that to log these warnings
@andrasmaroy Following python's library logging guide:
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.
Also, regarding logging.warning vs warnings.warn:
warnings.warn() in library code if the issue is avoidable and the client application should be modified to eliminate the warning logging.warning() if there is nothing the client application can do about the situation, but the event should still be noted
This is exactly the case for my PR. The warning produced by Pconf is nothing more than a log saying "you tried to load a file but it doesn't exist, so I am ignoring it".
I don't think forcing logs is the correct approach, and what I tried doing is exactly that: allowing the user to control whether he wants pconf
logs or not
When looking at it again, I think the captureWarnings
call is wrong and shouldn't be part of the library, as this affect the client's application.
WDYT about changing all those warnings to logger.warning
or even logger.info
?
Then each application can choose whether it wants those warnings or not.
Thank you @ronlut for taking the time for the thorough description and referencing sources!
Yes we are on the same page on this topic, I don't want to set handlers in the library, the guideline is absolutely right on that topic.
My issue was exactly as you said with captureWarnings
as that resulted in the warnings disappearing if one doesn't have a logger set up.
Changing the warnings the warnings to logger.warning
should be good, I think it aligns with the guide as in this case its not the application itself that should be modified necessarily to remedy the warning, but rather the environment its ran in.
@andrasmaroy removed the capturing + changed to logger. Now, for the following code:
import logging
from pconf import Pconf
Pconf.file("non_existing_file_1")
logging.basicConfig(level=logging.INFO, format='%(asctime)s %(levelname)-8s %(name)-30s %(message)s')
Pconf.file("non_existing_file_2")
The output is:
2021-07-22 13:43:23,362 WARNING pconf.store.file IOError when opening non_existing_file_2