fix(log): add logger for kazoo.handlers.threading

[zqfan]

Fixes #724

No handlers could be found for logger "kazoo.handlers.threading"

for better logging control, just like kazoo.client does

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 96.65%. Comparing base (92bd0c2) to head (0302623). Report is 16 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #725      +/-   ##
==========================================
- Coverage   96.73%   96.65%   -0.09%     
==========================================
  Files          27       27              
  Lines        3556     3560       +4     
==========================================
+ Hits         3440     3441       +1     
- Misses        116      119       +3     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[ceache]

Hi @zqfan. Thank you for the PR but, I am sorry but I am not clear on the goals of this PR. What is gained by making the handler reuse the client's logger? Do you have a clear example?

[zqfan]

Hi @zqfan. Thank you for the PR but, I am sorry but I am not clear on the goals of this PR. What is gained by making the handler reuse the client's logger? Do you have a clear example?

https://github.com/python-zk/kazoo/blob/b00d88ff1485f1cb175565c49955bf5d7ce96fef/kazoo/handlers/threading.py#L121 This line needs log to be set, i.e., logging.basicConfig(), otherwise, it might print an error message on stderr: "No handlers could be found for logger "kazoo.handlers.threading"" . As issue https://github.com/python-zk/kazoo/issues/724 pointed out, it leads to some information unnoticed, and cause a lot of time to dig out.

But it might be better if wen can specify a logger handler, which allows us to print log info into a specific file. Upper level application can only deal with kazoo.client, and it is not a good idea to expose kazoo/handlers/threading.py detail to upper level just for a logger handler, so I let it inherit the logger setting in kazoo.client.

this pr keeps same behavior as before, if user don't specify a dedicated logger handler

[StephenSorriaux]

Thank you for the PR.

Just like @ceache said, I am also not sure about the goal of this PR.

I understand the issue about the No handlers could be found for {logger} (which is explained in the kazoo documentation) that, I think, can be solved using a default NullHandler() for our top logger (as per the official documentation).

However, I don't think passing a logger should be the way to go.

[zqfan]

Thank you for the PR.

Just like @ceache said, I am also not sure about the goal of this PR.

I understand the issue about the No handlers could be found for {logger} (which is explained in the kazoo documentation) that, I think, can be solved using a default NullHandler() for our top logger (as per the official documentation).

However, I don't think passing a logger should be the way to go.

My project doesn't configure a logging.basicConfig for everything, it adjust and control major module's behaviour separately. For now, to avoid too much message printed to my log file, I have to do something like this:

logger = logging.getLogger("kazoo.client")
logger.setLevel(logging.ERROR)  // for other module, it is INFO
for h in mymodule.logger.handlers:  // a different file for other module
    logger.addHandler(h)
self.client = KazooClient(hosts=ZK_HOSTS,logger=logger)

If kazoo.handlers.threading could not inherit logger settings from kazoo client, then I have to do it twice. I think end user should treat kazoo as a integrated module, settings should be done only once.

[StephenSorriaux]

If kazoo.handlers.threading could not inherit logger settings from kazoo client, then I have to do it twice. I think end user should treat kazoo as a integrated module, settings should be done only once.

That can already be done: kazoo.handlers.threading does not inherit from kazoo.client but both inherit from kazoo, ie.

import logging
logging.basicConfig(level=logging.INFO)
...
logger = logging.getLogger("kazoo")
logger.setLevel(logging.ERROR)

@ceache I think the possible use case for this, which is not explained here or in the initial issue, is the same as the reason why we have a logger configuration in the client (see https://github.com/python-zk/kazoo/pull/85): passing a logger around to possibly discriminate between multiple instances of kazoo running on the same host or whatever. I must say I am... intrigued: isn't that something the developer must take care during the development (via the formatter, etc.)? boto3 or requests, to take some "famous" libs, do not let developers pass around their logger and I can easily understand why since Python already provides everything for that... Anyway, I would love your thoughts on this.

[jeffwidman]

I understand what you're trying to achieve, but as @StephenSorriaux points out above, I think it's already solvable if you change the logger you're tuning to kazoo rather than kazoo.client.

May I recommend @brandon-rhodes excellent logging_tree library? It's been a massive time saver for me over the years when I'm debugging unfamiliar code.

Closing, but if for some reason you can't achieve this with existing tooling please comment and we can re-evaluate.