classification
Title: loggers can't be pickled
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: vinay.sajip Nosy List: pitrou, serhiy.storchaka, vinay.sajip
Priority: normal Keywords:

Created on 2017-05-31 07:50 by pitrou, last changed 2017-06-06 15:39 by vinay.sajip. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 1956 merged vinay.sajip, 2017-06-05 12:41
Messages (7)
msg294818 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-05-31 07:50
Loggers could simply be pickled and unpickled by name, but pickle currently tries the hard way:

>>> import pickle
>>> import logging
>>> log = logging.getLogger('foo')
>>> pickle.dumps(log)
Traceback (most recent call last):
  File "<ipython-input-4-6ecead831873>", line 1, in <module>
    pickle.dumps(log)
TypeError: can't pickle _thread.RLock objects
msg294907 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-06-01 07:14
I am not sure it is a good idea to support pickling of loggers, as they are singletons and generally aren't supposed to be instantiated other than by calling logging.getLogger(name). What's the use case for pickling them, giving that (as you say) just the name could be used? In general, it's not needed to have loggers as part of an object instance, so I don't quite see where the need to pickle comes from.

One could implement __getstate__() to just return the name, but there is no corresponding obvious implementation of __setstate__() because loggers aren't meant to be instantiated via unpickling.
msg294908 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-06-01 07:18
Le 01/06/2017 à 09:14, Vinay Sajip a écrit :
> 
> I am not sure it is a good idea to support pickling of loggers, as they are singletons and generally aren't supposed to be instantiated other than by calling logging.getLogger(name).

Pickling them by name is precisely what I'm having in mind.  Right now,
pickle tries to recreate them structurally, which fails.

In other words (untested, but you get the idea):

  class Logger:
      def __reduce__(self):
          return getLogger, (self.name,)

> What's the use case for pickling them,

You usually don't pickle loggers directly.  You pickle an object that
happens to hold (directly or indirectly) a reference to a logger (it's
quite common to have `self.logger = ...` in your code), and pickle tries
to pickle the logger as part of pickling that object.

> One could implement __getstate__() to just return the name, but there
is no corresponding obvious implementation of __setstate__() because
loggers aren't meant to be instantiated via unpickling.

No need for __getstate__ or __setstate__, __reduce__ should work fine
(see above).
msg294921 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-01 09:28
The idea LGTM. But we should first check that the logger is accessible by the name: getLogger(self.name) is self. The same is done when pickling classes, functions, etc. It shouldn't be a surprise on unpickler side.

And maybe use not getLogger(), but different method which should fail if the logger doesn't exist rather than creating it. Otherwise this looks as pickling open files by name.
msg294923 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2017-06-01 09:38
Le 01/06/2017 à 11:28, Serhiy Storchaka a écrit :
> 
> The idea LGTM. But we should first check that the logger is accessible by the name: getLogger(self.name) is self. The same is done when pickling classes, functions, etc. It shouldn't be a surprise on unpickler side.

Good idea.

> And maybe use not getLogger(), but different method which should fail if the logger doesn't exist rather than creating it.

I disagree. The fact that it creates a new logger if it doesn't exist is
pretty much by design.  Typically, if you have:

class MyObject:
    def __init__(self):
        self.logger = getLogger('myobject')

then unpickling the first MyObject instance in a new process should also
create the 'myobject' logger instead of failing.
msg294930 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-06-01 10:27
> The fact that it creates a new logger if it doesn't exist is
> pretty much by design.

May be. I don't have a strong opinion.
msg295265 - (view) Author: Vinay Sajip (vinay.sajip) * (Python committer) Date: 2017-06-06 15:34
New changeset 6260d9f2039976372e0896d517b3c06e606eb169 by Vinay Sajip in branch 'master':
bpo-30520: Implemented pickling for loggers. (#1956)
https://github.com/python/cpython/commit/6260d9f2039976372e0896d517b3c06e606eb169
History
Date User Action Args
2017-06-06 15:39:08vinay.sajipsetstatus: open -> closed
assignee: vinay.sajip
resolution: fixed
stage: needs patch -> resolved
2017-06-06 15:34:32vinay.sajipsetmessages: + msg295265
2017-06-05 12:41:01vinay.sajipsetpull_requests: + pull_request2027
2017-06-01 10:27:12serhiy.storchakasetmessages: + msg294930
2017-06-01 09:38:45pitrousetmessages: + msg294923
2017-06-01 09:28:17serhiy.storchakasetnosy: + serhiy.storchaka

messages: + msg294921
stage: needs patch
2017-06-01 07:18:31pitrousetmessages: + msg294908
2017-06-01 07:14:07vinay.sajipsetmessages: + msg294907
2017-05-31 07:50:48pitroucreate