classification
Title: Adding support for setting the "disabled" attribute of loggers from logging.config.dictConfig
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: rejected
Dependencies: Superseder:
Assigned To: Nosy List: maggyero, piotr.dobrogost, vinay.sajip
Priority: normal Keywords: patch

Created on 2019-03-16 18:35 by maggyero, last changed 2020-10-13 15:10 by piotr.dobrogost. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 12372 closed maggyero, 2019-03-16 18:36
Messages (8)
msg338092 - (view) Author: Géry (maggyero) * Date: 2019-03-16 18:35
In the logging Python library, one can completely disable logging (for all levels) for a particular logger either by setting its `disabled` attribute to `True`, or by adding to it a `lambda record: False` filter, or by adding to it a `logging.NullHandler()` handler (to avoid the `logging.lastResort` handler) and setting its `propagate` attribute to `False` (to avoid log record propagation to its parent loggers):


import logging

# 1st solution
logging.getLogger("foo").disabled = True

# 2nd solution
logging.getLogger("foo").addFilter(lambda record: False)

# 3rd solution
logging.getLogger("foo").addHandler(logging.NullHandler())
logging.getLogger("foo").propagate = False


One can obtain the same logging configuration for the 2nd and 3rd solutions with the `logging.config.dictConfig` function:


import logging.config

# 2nd solution
logging.config.dictConfig({
    "version": 1,
    "filters": {
        "all": {
            "()": lambda: (lambda record: False)
        }
    },
    "loggers": {
        "foo": {
            "filters": ["all"]
        }
    }
})

# 3rd solution
logging.config.dictConfig({
    "version": 1,
    "handlers": {
        "null": {
            "class": "logging.NullHandler"
        }
    },
    "loggers": {
        "foo": {
            "handlers": ["null"],
            "propagate": False
        }
    }
})


However it is currently not possible for the 1st solution:


import logging.config

# 1st solution
logging.config.dictConfig({
    "version": 1,
    "loggers": {
        "foo": {
            "disabled": True
        }
    }
})

What do you think about adding this feature? I think it might be very convenient and improve consistency.
msg338157 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-03-18 02:46
I don't see any value in configuring a logger which starts as disabled - making it completely useless - why would one want to do this? I don't think the "consistency" argument flies in this case.
msg338175 - (view) Author: Géry (maggyero) * Date: 2019-03-18 08:33
Actually people do this all the time, to deactivate the logging of some third-party libraries (me included). For instance:

* https://stackoverflow.com/questions/24344045/how-can-i-completely-remove-any-logging-from-requests-module-in-python
* https://stackoverflow.com/questions/34598952/how-to-disable-info-logging-from-a-third-party-module-in-python
* https://stackoverflow.com/questions/38102291/turn-off-logging-in-schedule-library

And currently we can only use either solution 2 or 3 with `logging.config.dictConfig` (which are more verbose and less explicit). We cannot use solution 1 with `logging.config.dictConfig`.

In addition, all public attributes in the `__init__` methods of the `logging.Formatter`, `logging.Handler` and `logging.Logger` classes can be set from `logging.config.dictConfig`, except the `disabled` attribute, which is inconsistent.
msg338231 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-03-18 14:36
> Actually people do this all the time, to deactivate the logging of some third-party libraries (me included).

It would be better to set the level of those loggers to e.g. ERROR or CRITICAL rather than disabling them altogether - presumably if something bad happens in those packages, one would want to know!

> In addition, all public attributes ... can be set from `logging.config.dictConfig`, except the `disabled` attribute, which is inconsistent.

I don't especially want people to use `disabled` for this type of thing - the main reason for having it is that following an on-the-fly reconfiguration in a long-running process, some threads might still be active that contain references to now-unwanted loggers, and disabling makes those references inactive without the need to track them down. I would recommend using e.g. CRITICAL as a level for the use case you mention.
msg338257 - (view) Author: Géry (maggyero) * Date: 2019-03-18 16:49
> It would be better to set the level of those loggers to e.g. ERROR or CRITICAL rather than disabling them altogether - presumably if something bad happens in those packages, one would want to know!

What if the user doesn't care anyway? Why assuming what the user wants? And with solutions 2 and 3, the user can already *completely* silence a specific logger from `logging.config.dictConfig`. So it cannot be the reason why the `disabled` attribute cannot be set from `logging.config.dictConfig`.

> I don't especially want people to use `disabled` for this type of thing - the main reason for having it is that following an on-the-fly reconfiguration in a long-running process, some threads might still be active that contain references to now-unwanted loggers, and disabling makes those references inactive without the need to track them down.

I think you are referring to the `disable_existing_loggers` key in `logging.config.dictConfig`, which sets the `disabled` attribute for loggers present in a previous configuration but absent in the new configuration (https://github.com/python/cpython/blob/master/Lib/logging/config.py#L161).

So a user can already deactivate *all* existing loggers from `logging.config.dictConfig`. What if he wants to deactivate only *one* specific logger, but not all (for whatever reason)?

If the `disabled` attribute should not be part of the public API, it should have been name `_disabled`. Currently everyone can type this anyway:


import logging

logging.getLogger("foo").disabled = True


So it should be doable from `logging.config.dictConfig` too in my opinion.
msg338278 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2019-03-18 18:21
> If the `disabled` attribute should not be part of the public API it should have been name `_disabled`.

A bit late for that now, and AFAIK it hasn't caused people insuperable problems heretofore - the code has been like this pretty much since the logging package was added to Python.


> So it should be doable from `logging.config.dictConfig` too in my opinion.

Let's just agree to disagree on this for now. It's certainly not a common use case, there are other ways to achieve the desired result in those uncommon cases, and the argument seems to come back to "consistency". I'd rather leave this as it is.
msg338660 - (view) Author: Géry (maggyero) * Date: 2019-03-23 09:08
> A bit late for that now

Okay, so disabled is a private attribute so it should never be used anyway. And as it not documented anywhere in the official Python documentation, I think the current situation is fine. Thank you for the explanation, I am closing this issue and the pull request.
msg378563 - (view) Author: Piotr Dobrogost (piotr.dobrogost) Date: 2020-10-13 15:10
I strongly agree with arguments given by the original poster. Stackoverflow's questions cited show the functionality of disabling logger is something people are looking for.
Disabling logger by setting high enough level seems to be a workaround at best (sys.maxsize - 
 really?). More in the spirit of an on/off toggle is a filter blocking all records but this clearly feels like another workaround for something which should have been (and in fact already is, albeit unofficially) a simple boolean flag.
Not hiding "disabled" property behind underscore might have been a good thing after all. Making it official and supported by dictConfig() would remove clearly expressed pain point of users of the logging module.
History
Date User Action Args
2020-10-13 15:10:44piotr.dobrogostsetmessages: + msg378563
2020-08-31 17:47:08piotr.dobrogostsetnosy: + piotr.dobrogost
2019-03-23 09:09:36maggyerosetstatus: open -> closed
resolution: rejected
stage: patch review -> resolved
2019-03-23 09:08:28maggyerosetmessages: + msg338660
2019-03-18 18:21:48vinay.sajipsetmessages: + msg338278
2019-03-18 16:49:09maggyerosetmessages: + msg338257
2019-03-18 14:36:08vinay.sajipsetmessages: + msg338231
2019-03-18 08:33:53maggyerosetmessages: + msg338175
2019-03-18 02:46:14vinay.sajipsetmessages: + msg338157
2019-03-16 18:36:04maggyerosetkeywords: + patch
stage: patch review
pull_requests: + pull_request12334
2019-03-16 18:35:29maggyerocreate