-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Allow configuring logger_class with statsd_host #1188
Conversation
This is a proposed fix for #1187 |
Is it the case that a user should probably not specify both arguments together? Should we issue a warning when both are provided? |
On 2 February 2016 at 23:37, Randall Leeds notifications@github.com wrote:
My use case is exactly that, though. I did look at adding a warning about makeing sure that your custom logger Thanks Simon Simon |
I think this is fine. What do you think about maybe where you have the "N.B." in the docs for the statsd option to add a note that any custom logger class specified should be API-compatible with the statsd logger. That seems like a reasonable place to document this and easier than trying to issue a warning. This is great, thank you. |
On 11 February 2016 at 21:26, Randall Leeds notifications@github.com
Thank you for taking the time to look at it :) Simon |
Currently if you configure statsd_host, a configured logger_class will never be used. I think this makes a user configured logger class always take priority. (This is PoC change, I will come back and add tests/docs if it's worth pursuing)
Previously, configuring statsd_host would override the configured class
4e28866
to
b05286e
Compare
Rebased on master, be great if you could land |
Beautiful. Thanks for your effort and patience, @bloodearnest! |
Allow configuring logger_class with statsd_host
Allow configuring logger_class with statsd_host
Currently if you configure statsd_host, a configured logger_class will never be used.
I think this makes a user configured logger class always take priority.
(This is PoC change, I will come back and add tests/docs if it's worth pursuing)