Chaffelson / nipyapi

A convenient Python wrapper for Apache NiFi
Other
243 stars 76 forks source link

root logger in config file causing duplicated message printed #339

Closed YongpengFu closed 3 months ago

YongpengFu commented 7 months ago

Description

There are duplicated log messages on the console when I use my own logger after importing nipyapi.

What I Did

I am using the following code snippet to show the output:

import logging
import nipyapi

# Cusomized logger
logger1 = logging.getLogger('myapp.area1')
logger1.setLevel(logging.DEBUG)

console = logging.StreamHandler()
formatter = logging.Formatter('%(name)-12s: %(levelname)-8s %(message)s')
console.setFormatter(formatter)
logger1.addHandler(console)

logger1.debug('Quick zephyrs blow, vexing daft Jim.')

What I am supposed to get is:

myapp.area1 : DEBUG    Quick zephyrs blow, vexing daft Jim.

What I end up getting:

myapp.area1 : DEBUG    Quick zephyrs blow, vexing daft Jim.
**DEBUG:myapp.area1:Quick zephyrs blow, vexing daft Jim.**

Reason

In nipyapi-->config.py, nipyapi's call to basicconfig has configured 'root' logger to catch events with warning level and above and send them to stderr. That effectively results in every event from every logger being processed. It is recommended that a library should not configure logging, only the application should do that.

Urgency

This is not super urgent, but it can cause a lot of headache for any developer who is using this package if they want to have their own logger.

Chaffelson commented 7 months ago

You make a good point, it does not currently follow Python best practices.

We could move to the standard NullHandler call in config.py, and then only call basicconfig in the demo scripts perhaps?

ottobackwards commented 7 months ago

I'm going to be honest, I think since you use getLogger(name) everywhere ( even the demo) I think you can just remove that line from config and set the null handler and be done

ottobackwards commented 7 months ago

I'm not even sure you should set the null handler

YongpengFu commented 7 months ago

Actually @ottobackwards is right. You are using getLogger(name) in everywhere, you don't really need a basicconfig in the config file. As a matter of fact, the issue will persist even when you use standard NullHandler, because a new streamhandler will still be created in the highest of the logger hierarchy (basically the root logger).

ottobackwards commented 7 months ago

I opened a PR