This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: LoggerAdapter example is counter-productive
Type: behavior Stage: resolved
Components: Documentation Versions: Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: docs@python Nosy List: docs@python, pitrou, python-dev, vinay.sajip
Priority: normal Keywords:

Created on 2013-07-24 09:27 by pitrou, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Messages (6)
msg193633 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-07-24 09:27
The idiomatic way to use a LoggerAdapter is to override the process() method. However, the example in the cookbook (*) is some gobbledegook code overriding __getitem__ and __iter__ on a separate class. It's a pity that users are directed to use an insane and impractical idiom while the sane way is simple and powerful.

(*) http://docs.python.org/2/howto/logging-cookbook.html#context-info
msg193644 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2013-07-24 13:45
That's not quite right. The recommended way *is* to override the process() method. From the page you linked to:

"If you need a different method, e.g. if you want to prepend or append the contextual information to the message string, you just need to subclass LoggerAdapter and override process() to do what you need."

That does not have a specific example, as it seems simple enough to understand as stated.

The example you mention shows something else - how you would adapt an existing class (which might have information to go into the log) so that it could be passed (instead of a dict) to the LoggerAdapter initialiser. Possibly I could put this example into a separate section "Adapting an existing class to provide context information for logging" - this would make it clear that it's a separate use case and not the primary use case when using LoggerAdapter. The text seems clear enough,

"illustrates what dict-like behaviour is needed from an arbitrary ‘dict-like’ object for use in the constructor", but splitting it out into a separate section will make it easier for those who are speed-reading the docs.
msg193645 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-07-24 13:57
> That does not have a specific example, as it seems simple enough to
> understand as stated.

I think we should acknowledge that people often have difficulties
with the rather simple functionalities of the logging module,
not only the advanced ones.

> The example you mention shows something else - how you would adapt an
> existing class (which might have information to go into the log) so
> that it could be passed (instead of a dict) to the LoggerAdapter
> initialiser.

But how is that necessary for the use case? Your LoggerAdapter-derived
class could take the "existing class" as a constructor parameter (*), then
inject the required info in its overriden process() method.

(*) either by overriding the constructor, or simply by passing the
"existing class" as an entry in the "extra" dict.

If I had trusted your doc blindly, I would have thought it necessary
to go through the "complicated scheme", while the simple scheme is 
actually sufficient to add per-connection info to log messages.
msg193649 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2013-07-24 14:33
> I think we should acknowledge that people often have difficulties
> with the rather simple functionalities of the logging module,
> not only the advanced ones.

Perhaps some people do have difficulties, but that's always going to be the case no matter what you do. A cookbook should explore different things, both simple and less simple.

> But how is that necessary for the use case? Your LoggerAdapter-
> derived class could take the "existing class" as a constructor
> parameter (*), then inject the required info in its overriden
> process() method.

If you can adapt an existing class to look sufficiently like a dict, that's all you need to do - there's no need to subclass LoggerAdapter and override process(). There might be cases where that's the easier option.

It's surprising how resistant people can be to subclassing and overriding. For example, for issue #18345 which you raised (which I haven't yet addressed as you said it was low-priority), one straightforward approach would be to subclass the relevant FileHandler classes.

> If I had trusted your doc blindly, I would have thought it necessary
> to go through the "complicated scheme"

It depends on how carefully you read it - I don't think it's *actually* misleading, and I quoted in my earlier response the sentence, which comes before the example, which says that overriding process() is what you'd normally do.

What about my suggestion about a separate section for the example, to make it clearer that it's just another approach which might be more suitable in some scenarios?
msg193653 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-07-24 14:49
> > But how is that necessary for the use case? Your LoggerAdapter-
> > derived class could take the "existing class" as a constructor
> > parameter (*), then inject the required info in its overriden
> > process() method.
> 
> If you can adapt an existing class to look sufficiently like a dict,
> that's all you need to do - there's no need to subclass
> LoggerAdapter and override process(). There might be cases where
> that's the easier option.

I am not talking about "adapting a class". I am taking about overriding
process() to do the adapting in a simpler way than in your example.
msg193664 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-07-24 16:53
New changeset 16c15d7d4480 by Vinay Sajip in branch '2.7':
Issue #18541: simplified LoggerAdapter example.
http://hg.python.org/cpython/rev/16c15d7d4480

New changeset adaecee37745 by Vinay Sajip in branch '3.3':
Issue #18541: simplified LoggerAdapter example.
http://hg.python.org/cpython/rev/adaecee37745

New changeset f931ee89cc1c by Vinay Sajip in branch 'default':
Closes #18541: merged update from 3.3.
http://hg.python.org/cpython/rev/f931ee89cc1c
History
Date User Action Args
2022-04-11 14:57:48adminsetgithub: 62741
2013-07-24 16:53:16python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg193664

resolution: fixed
stage: resolved
2013-07-24 14:49:52pitrousetmessages: + msg193653
2013-07-24 14:33:27vinay.sajipsetmessages: + msg193649
2013-07-24 13:57:05pitrousetmessages: + msg193645
2013-07-24 13:45:47vinay.sajipsetmessages: + msg193644
2013-07-24 09:27:48pitroucreate