Opened 4 years ago

Closed 4 years ago

#6087 release blocker: regression closed fixed (fixed)

Restore (and test more reliabley) twisted.python.log's capture of warnings

Reported by: itamar Owned by: itamar
Priority: normal Milestone: Python 3.3 Minimal
Component: core Keywords:
Cc: exarkun, ralphm Branch: branches/warnings-6087
branch-diff, diff-cov, branch-cov, buildbot
Author: itamarst


Warnings are supposed to end up in the Twisted log once logging is initialized, but #5885 broke that functionality. It should be fixed (reverting #5885 would be somewhat painful at this point) and the tests improved so this doesn't happen again.

Change History (7)

comment:1 Changed 4 years ago by itamarst

  • Author set to itamarst
  • Branch set to branches/warnings-6087

(In [36022]) Branching to 'warnings-6087'

comment:2 Changed 4 years ago by itamar

  • Cc exarkun ralphm added
  • Keywords review added
  • Owner itamar 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.

comment:3 Changed 4 years ago by itamar

  • Type changed from defect to regression

comment:4 Changed 4 years ago by exarkun

  • Keywords review removed
  • Owner set to itamar


  1. in test_publisherWarningInitialization
    1. the builtin setattr is preferable to using warnings.__setattr__.
  2. twisted/python/
    1. 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).

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 4 years ago by itamar

  • Keywords review added
  • Owner changed from itamar to exarkun 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 4 years ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to itamar

Okay. Please merge.

comment:7 Changed 4 years ago by itamarst

  • Resolution set to fixed
  • Status changed from new to closed

(In [36349]) Merge warnings-6087.

Author: itamar Review: exarkun Fixes: #6087

The Python 3 port of logging mistakenly broke warning integration; this fixes it. It's a .misc because from point of view of upgrading user nothing will have changed.

Note: See TracTickets for help on using tickets.