bpo-30520: Implemented pickling for loggers. #1956
Conversation
Doc/library/logging.rst
Outdated
|
|
||
| .. versionadded:: 3.2 | ||
|
|
||
| .. versionadded:: 3.7 |
There was a problem hiding this comment.
That should probably be a versionchanged.
Lib/test/test_logging.py
Outdated
| def test_pickling(self): | ||
| for name in ('', 'root', 'foo', 'foo.bar', 'baz.bar'): | ||
| logger = logging.getLogger(name) | ||
| s = pickle.dumps(logger) |
There was a problem hiding this comment.
Please test it for all protocols:
for proto in range(pickle.HIGHEST_PROTOCOL + 1):
s = pickle.dumps(logger, proto)
Lib/logging/__init__.py
Outdated
| Logger.__init__(self, "root", level) | ||
|
|
||
| def __reduce__(self): | ||
| return getLogger, ('',) |
There was a problem hiding this comment.
Maybe without arguments at all?
return getLogger, ()
| def __reduce__(self): | ||
| # In general, only the root logger will not be accessible via its name. | ||
| # However, the root logger's class has its own __reduce__ method. | ||
| if getLogger(self.name) is not self: |
There was a problem hiding this comment.
This implicitly creates the global logger with name self.name. Is it okay?
There was a problem hiding this comment.
I don't see a problem with it.
| # In general, only the root logger will not be accessible via its name. | ||
| # However, the root logger's class has its own __reduce__ method. | ||
| if getLogger(self.name) is not self: | ||
| raise pickle.PicklingError('logger cannot be pickled') |
There was a problem hiding this comment.
The pickle module is used only here. It isn't used if pickling is not involved. May be make an exception from PEP 8 and import it locally?
| self.assertRaises(TypeError, logging.getLogger, any) | ||
| self.assertRaises(TypeError, logging.getLogger, b'foo') | ||
|
|
||
| def test_pickling(self): |
There was a problem hiding this comment.
__reduce__ is used also in the copy module. Maybe add tests for copy.copy() and copy.deepcopy()? Even unpickleable loggers can be copied if add special __copy__ and __deepcopy__ methods or register the classes with copyreg.pickle().
There was a problem hiding this comment.
Loggers are singletons, so copy operations shouldn't actually make copies, anyway. Seems like a separate issue (or at least use case).
Lib/test/test_logging.py
Outdated
| self.assertRaises(TypeError, logging.getLogger, b'foo') | ||
|
|
||
| def test_pickling(self): | ||
| for name in ('', 'root', 'foo', 'foo.bar', 'baz.bar'): |
There was a problem hiding this comment.
Is self.logger pickleable? If not, it may make sense to add an explicit test for this.
There was a problem hiding this comment.
No, it's not pickleable, as it's created using Logger.__init__(), which users are not supposed to use.
There was a problem hiding this comment.
It is created via Logger constructor, not via getLogger(). logging.Logger('blah') is not logging.getLogger('blah').
|
Ah, just please add a Misc/NEWS entry. |
Also weakens the 'should this be run?' regex to allow all builds when .travis.yml changes.
GH-1439) Several class attributes have been added to calendar.HTMLCalendar that allow customization of the CSS classes used in the resulting HTML. This can be done by subclasses HTMLCalendar and overwriting those class attributes (Patch by Oz Tiram).
* Simplify X.509 extension handling code The previous implementation had grown organically over time, as OpenSSL's API evolved. * Delete even more code
No description provided.