Opened 6 years ago
Closed 5 years ago
#6087 release blocker: regression closed fixed (fixed)
Restore (and test more reliabley) twisted.python.log's capture of warnings
Reported by: | Itamar Turner-Trauring | Owned by: | Itamar Turner-Trauring |
---|---|---|---|
Priority: | normal | Milestone: | Python 3.3 Minimal |
Component: | core | Keywords: | |
Cc: | Jean-Paul Calderone, ralphm | Branch: |
branches/warnings-6087
branch-diff, diff-cov, branch-cov, buildbot |
Author: | itamarst |
Change History (7)
comment:1 Changed 6 years ago by
Author: | → itamarst |
---|---|
Branch: | → branches/warnings-6087 |
comment:2 Changed 6 years ago by
Cc: | Jean-Paul Calderone ralphm added |
---|---|
Keywords: | review added |
Owner: | Itamar Turner-Trauring deleted |
I've fixed it, I think. Note that this involves a minor change in the way we do things; when we write a warning to a file in LogPublisher.showwarning
, we use the *original* warnings.showwarning
(or at least, what was there at import time). Previously we used the version captured by startLoggingWithObserver
, which caused problems using LogPublisher.showwarning
before logging had been initialized. It is vaguely possible this will break some obscure code somewhere that overrides showwarning sufficiently to actually write to files differently, and does the override in between importing twisted.python.log
and starting twisted logging. I am inclined not to care, but I could come up with a different solution.
http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/warnings-6087
comment:3 Changed 6 years ago by
Type: | defect → regression |
---|
comment:4 Changed 5 years ago by
Keywords: | review removed |
---|---|
Owner: | set to Itamar Turner-Trauring |
Thanks.
- in
test_publisherWarningInitialization
- the builtin
setattr
is preferable to usingwarnings.__setattr__
.
- the builtin
twisted/python/log.py
- As discussed on IRC, the
showwarning
APIs here aren't intended to be used by applications. I think we should move towards deprecating them and not worry about all the super obscure edge cases in their behavior (related to what order events happen in, etc).
- As discussed on IRC, the
Not sure many more review comments make sense, given that hopefully the last point above means things will change significantly and for the better.
comment:5 Changed 5 years ago by
Keywords: | review added |
---|---|
Owner: | changed from Itamar Turner-Trauring to Jean-Paul Calderone |
http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/warnings-6087 has started.
I implemented a simpler "fix"; we're back to the previous behavior from before the Python 3 changes, I believe, with some tests skipped or munged until they work.
comment:6 Changed 5 years ago by
Keywords: | review removed |
---|---|
Owner: | changed from Jean-Paul Calderone to Itamar Turner-Trauring |
Okay. Please merge.
comment:7 Changed 5 years ago by
Resolution: | → fixed |
---|---|
Status: | new → closed |
(In [36022]) Branching to 'warnings-6087'