ultimate-sitemap-parser
ultimate-sitemap-parser copied to clipboard
mediacloud
Library interferes with application logging configuration
Because of this:
https://github.com/mediacloud/ultimate-sitemap-parser/blob/68d1ccdd573be16cdfadc7f071c088642182bff1/usp/fetch_parse.py#L43
and this:
https://github.com/mediacloud/ultimate-sitemap-parser/blob/68d1ccdd573be16cdfadc7f071c088642182bff1/usp/log.py#L36-L43
this library almost always ends-up configuring the logging of the calling application in ways not often desired. It's usually not appropriate to unconditionally print logging to the screen. Can you please remove the code (shown above) where it's adding handlers if no handlers already exists?
Also, this module is overriding all of the built-in logging functionality unnecessarily. It could just use the logging object directly (e.g. _LOGGING = logging.getLogger(__name__)), but that's just a side comment.
I'm currently having to do this to squash your output:
import usp.fetch_parse
import usp.objects.sitemap
logging.getLogger('usp.fetch_parse').handlers = []
logging.getLogger('usp.helpers').handlers = []
Could you do it in a PR?
On Fri, Nov 20, 2020, 01:41 Dustin Oprea [email protected] wrote:
Because of this:
https://github.com/mediacloud/ultimate-sitemap-parser/blob/68d1ccdd573be16cdfadc7f071c088642182bff1/usp/fetch_parse.py#L43
and this:
https://github.com/mediacloud/ultimate-sitemap-parser/blob/68d1ccdd573be16cdfadc7f071c088642182bff1/usp/log.py#L36-L43
this library almost always ends-up reconfiguring the logging of the calling application. Because it's running as soon as the module is imported, it's configuring the logging before the host application has a chance to. That aside, a package/library should just be using the logging, not configuring the logging. That's the responsible of the front-end scripts/application. It's disruptive, otherwise. It's usually not appropriate to unconditionally print logging to the screen.
Can you please remove the code (shown above) where it's adding handlers to the current logging config if no handlers already exists?
Also, this module is overriding all of the built-in logging functionality unnecessarily. It could just use the logging object directly (e.g. _LOGGING = logging.getLogger(name)), but that's just a side comment.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mediacloud/ultimate-sitemap-parser/issues/23, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABMNPKHPULO2DGHQ72II73SQWUJHANCNFSM4T4CCXLA .
Sure. I just wanted to see if you'd respond, first.
https://github.com/mediacloud/ultimate-sitemap-parser/pull/24
A bit busy this week, I'll try to get to it soon though.
On Mon, Nov 23, 2020 at 4:56 AM Dustin Oprea [email protected] wrote:
Bump
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mediacloud/ultimate-sitemap-parser/issues/23#issuecomment-731901848, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABMNPN36KVPEJV5DWEVRSTSRHFOTANCNFSM4T4CCXLA .
-- Linas Valiukas www.mediacloud.org
Hey @pypt - We need this change in our app too. Do you have a rough idea of when the change will be added?
