-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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.config.dictConfig does not work with callable filters #86072
Comments
According to the docs here (https://docs.python.org/3/library/logging.html): "You don’t need to create specialized Filter classes, or use other classes with a filter method: you can use a function (or other callable) as a filter. The filtering logic will check to see if the filter object has a filter attribute: if it does, it’s assumed to be a Filter and its filter() method is called. Otherwise, it’s assumed to be a callable and called with the record as the single parameter." If I use this code: def noErrorLogs(param):
return 1 if param.levelno < 40 else 0
logconfig_dict = {
'filters': {
'myfilter': {
'()': noErrorLogs,
}
},
"handlers": {
"console": {
"class": "logging.StreamHandler",
"level": "DEBUG",
"stream": "ext://sys.stdout",
"filters": ["myfilter"]
}
},
"root": {"level": "DEBUG", "handlers": ["console"]},
"version": 1,
}
dictConfig(logconfig_dict) I get the error "Unable to configure filter 'myfilter'" because "noErrorLogs() missing 1 required positional argument: 'param'" However, If I use this code: def noErrorLogs(param):
return 1 if param.levelno < 40 else 0
logconfig_dict = {
"handlers": {
"console": {
"class": "logging.StreamHandler",
"level": "DEBUG",
"stream": "ext://sys.stdout",
}
},
"root": {"level": "DEBUG", "handlers": ["console"]},
"version": 1,
}
logging.basicConfig()
dictConfig(logconfig_dict)
l = logging.getLogger()
l.handlers[0].addFilter(noErrorLogs) Then the filter works correctly. The bug I am reporting is that when using logging.config.dictConfig you cannot pass in a callable that acts a filter. You can only pass in a class with a filter method or a function that returns a callable filter. If this is the expected behavior perhaps the docs should make it clear that callables cannot be used with dictConfig. |
The value of the '()' key should be a factory - something that returns a filter (either something that has a filter method or a callable). That returned callable, or the filter method, will be called with a LogRecord and its return value used to determine whether the event should be processed. An example of using factories is here: https://docs.python.org/3/library/logging.config.html#user-defined-objects In your first example, you're passing the filter where you need to pass the factory. If you change it to e.g. return a lambda that does the filtering, it should work as expected. At the moment you can't directly pass a filter callable in the config dictionary - only a factory that returns a filter. I'll look at this as a possible enhancement. |
Thank you for the clarification. I think I was most confused by the docs on this page (which I should have included in my initial post): https://docs.python.org/3/howto/logging.html It says: "In Python 3.2, a new means of configuring logging has been introduced, using dictionaries to hold configuration information. This provides a superset of the functionality of the config-file-based approach outlined above, and is the recommended configuration method for new applications and deployments." Since it is the recommended configuration method I had naively assumed that it would be compatible with the feature of just using callables instead which was mentioned here https://docs.python.org/3/library/logging.html. I think it would be a nice enhancement long term to be able too support callables in dictconfigs. In the short term do you think it is reasonable to update the first page mentioned in this comment to clarify that even though dictConfig is recommended it is not a feature parity with the object oriented api? It would also be nice to clarify this on the second page to say that callables don't work if using dictConfig. I understand it does say a factory is required on the page you linked. I think anyone who reads the docs as well as they should will find it. But I think adding information about this too other parts of the docs will make it much easier for newer folks to catch the difference early on. Thanks for your work on the logging parts of the api in python. I've read many of your docs and they are very helpful. |
You make reasonable points. I won't close this issue, and get to those updates when I can/as time allows. Meanwhile, if you or someone else proposes specific changes by way of a pull request, I'll look at that too. And thanks for your kind words about the logging docs. |
I looked into implementing this and it's not entirely clear how it could work. The main issue I encountered is how to distinguish between a callable that is a factory that takes one argument and a callable that is a filter. One possible approach I see is to rely on the absence of any other keys in the configuration sub-directory. Not entirely happy with that (and have to think how to treat factory with optional parameter vs filter with optional parameter). A different approach that I think would be not optimal is to just call the callable with a dummy LogRecord passed ind and see if it returns a boolean or callable (or throws). |
Vinay would you consider a patch for logging where dictConfig allows taking objects directly in addition to the reference id? That would allow the following:
or alternatively passing them on declaration:
I'm happy to put a patch together if that looks good to you. |
I would definitely vote for implementing this enhancement. I have just ran into the very same issue and my search ended here. Using dictConfig e.g. with lambdas seems very natural to me and I understood the docs incorrectly exactly as had been reported. |
Sorry I didn't respond to this; it dropped off my radar. Certainly, I would consider such a patch. |
Great! I'll put something together then. If you have any preference about the implementation or any pointer on the way you think should be done, please let me know. |
Nothing particular, just try to fit in with the existing code conventions even where they don't follow modern idioms (e.g. a lot of the code in this package predates PEP-8 and new styles of string formatting). |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: