#2244 Improve the logging situation
Merged by pingou. Opened by pingou.
logging  into  master

Download 2244.patch

Place a logging configuration dictionary in pagure's configuration file,
use module specific logger and drop the use of the logger object shared
between modules.

Signed-off-by: Pierre-Yves Chibon pingou@pingoured.fr

This might work (I honestly don't know), but you should use the 'root' key to configure the root logger. Anitya has an example.

Is there a reason you're using the flask logger rather than a module logger?

Sure, will adjust

I thought it was the proper way, I can update if you think it's a bad idea :)

rebased

That Flask logger is simply a logger called whatever you called the Flask app (which is recommended to be name IIRC). I'm not sure how familiar you are with the details of the Python logging module, but when you call logging.getLogger(name) with the same string, you get the same object:

>>> import logging
>>> logging.getLogger('pagure') is logging.getLogger('pagure')
True

This means you can get access to that same logger instance easily:

>>> import logging
>>> from anitya.app import APP
>>> APP.logger is logging.getLogger('anitya.app')
True
>>> APP.logger is logging.getLogger('anitya.config')
False

However, I don't recommend you do that. Instead, do what you've done everywhere else (_log = logging.getLogger(__name__)). Why, you might ask? Well, it has to do with how the logging configuration works.

You apply a logging configuration to a package, sub-package, or module (in your configuration, you've applied it to the top level packages only). Suppose I want debug level logging statements for the pagure.lib.login module, but warning level logging from the the rest of the code in pagure. I would set the logger dictionary to look like:

    'loggers': {
        'pagure': {
            'handlers': ['console'],
            'level': 'INFO',
            'propagate': False
        },
        'pagure.lib.login': {
            'handlers': ['console'],
            'level': 'DEBUG',
            'propagate': False
        },

However, if the pagure.lib.login module uses a logger called pagure, this won't work. If there's not a log configuration for a module, the package a module is in is checked for a configuration. If that doesn't have a configuration, its parent package (if any) is checked. This continues recursively until the root logger is hit or a config is found that has 'propagate' set to False. Here's a nice little diagram of the logging flow.

TL;DR:
Use _log = getLogger(__name__) to allow for fine-grain logging configuration.

This needs to be outside the loggers dict - it's a special keyword. I assume they did this so there can be a Python package called 'root'.

Are you sure you want to propagate the logs? This will pass the log to all handlers associated with all ancestor loggers (in this case, just the root logger)

Makes sense to me, I'll adjust, thanks :)

I actually thought propagate was about going down stream not up stream. IE, using propagate: True on the pagure loggers would propagate that to pagure.app pagure.lib.git & so on.

If it does the opposite, then I most definitively don't want it by default :)

rebased

Pretty please pagure-ci rebuild

Looks good :thumbsup:

Thanks! :)

Pull-Request has been merged by pingou

Metadata