Closed gbrindisi closed 9 years ago
Hi, @gbrindisi ! Thanks a lot for this. We require first time contributors to sign a Contributor License Agreement. Can I send it to you by e-mail so you can review?
@alexcpsec sure: g # brindi , si
Looking at this, the first thing I notice is that it assumes the various components (reaper.py
thresher.py
etc.) will only be called from combine.py
. But that may not happen that way. Users, including me, often need to call the various components individually, e.g. just testing the enrichment functions. And in those cases, logging is perhaps even more important than usual because that's often in support of debugging.
How would you suggest that we allow them to use the proper logging facilities even when called in that fashion?
@technoskald you are right I didn't think about this use case, sorry.
Then it should be better to use hierarchical loggers for every module.
logger.getLogger('combine.reaper');
After having defined the root 'combine' logger with all his handlers. Loggers down in the hierarchical list will inherit the level and the handlers, if I recall correctly. So it should be a matter of abstracting away the root logging initialization part and call it once every module startup - the logging library does not create new loggers if already defined, nor it will attach the same handler two times.
This is just the first thing off the top my head, I'll try to put it in code later today if it sounds good to you
I like this option if it works like you described.
I've refactored to use hierarchical loggers for every module.
The logger initalization function has been abstracted away and now checks if the root logger has the proper stream handlers attached, if not they are created. Finally the module-level logger is returned which inherit all the configuration from the root one.
Awesome, can't wait to review.
Superseded by #83
Here you go! #34