#6087 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
(diff, github, buildbot, log)
Author: itamarst Launchpad Bug:

Description

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 23 months ago by itamarst

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

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

comment:2 Changed 23 months 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.

http://buildbot.twistedmatrix.com/boxes-supported?branch=/branches/warnings-6087

comment:3 Changed 23 months ago by itamar

  • Type changed from defect to regression

comment:4 Changed 22 months ago by exarkun

  • Keywords review removed
  • Owner set to itamar

Thanks.

  1. in test_publisherWarningInitialization
    1. the builtin setattr is preferable to using warnings.__setattr__.
  2. twisted/python/log.py
    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 22 months ago by itamar

  • Keywords review added
  • Owner changed from itamar to exarkun

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 22 months ago by exarkun

  • Keywords review removed
  • Owner changed from exarkun to itamar

Okay. Please merge.

comment:7 Changed 22 months 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.