[Patches] [ python-Patches-1484758 ] cookielib: reduce (fatal) dependency on "beta" logging?

[SourceForge.net]

Patches item #1484758, was opened at 2006-05-09 15:14
Message generated for change (Comment added) made by gbrandl
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1484758&group_id=5470

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Library (Lib)
Group: Python 2.4
Status: Closed
Resolution: Fixed
Priority: 5
Submitted By: kxroberto (kxroberto)
Assigned to: Nobody/Anonymous (nobody)
Summary: cookielib: reduce (fatal) dependency on "beta" logging?

Initial Comment:
The logging package is tagged "beta". Yet cookielib (as
the ONLY module in the std. lib !?) uses Logger.debug()
very excessively.

I got occasional nasty crash traces (from users) when
using cookielib Processors through urllib2
(multi-threaded usage) - see below.  The causes are not
errors in cookielib, but upon simple calls to
Logger.debug() : varying AttributeError's in logging,
which on the first glance seem to be impossible, as
those attributes are set in the related __init__()'s
but there are strange complex things going on with
roots/hierarchies/copy etc. so....  thread/lock
problems I'd guess.

the patch uncomments several debug() calls in cookielib
in import. only one's in important high-frequency
execution flow path (not ones upon errors and
exceptional states). And 2 minor fixes on pychecker
warnings.

After applying that, the nasty crash reports disappeared.

I do not understand completely why the cookielib
production code has to use the logging package
(expensive) at all. At least for the high-frq used
add_cookie_header its unnecessary. There could be some
simpler (detached) test code for testing purposes.
Importing the logging and setup is time consuming etc.
(see other patch for urllib2 import optimization. )

I'd recommend: At least as far as logging is "beta" and
cookielib NOT, all these debug()'s should be
uncommented, or at least called ONLY upon a dispatching
global 'use_logging' variable in cookielib, in case the
test code cannot be externalized nicely.


2 example error traces:

...File "cookielib.pyo",
line 1303, in add_cookie_header\\n\', \'  File
"logging\\\\__init__.pyo", line 878, in debug\\n\',
\'  File "logging\\\\__init__.pyo", line 1056, in
getEffectiveLevel\\n\', "AttributeError: Logger
instance has no attribute \'level\'\\n



...in http_request\\n\', \'  File "cookielib.pyo", line
1303, in add_cookie_header\\n\', \'  File
"logging\\\\__init__.pyo", line 876, in debug\\n\',
"AttributeError: Manager instance has no attribute
\'disable\'\\n


-robert

----------------------------------------------------------------------

>Comment By: Georg Brandl (gbrandl)
Date: 2006-05-18 06:21

Message:
Logged In: YES 
user_id=849994

As long as only one standard module uses logging, it's quite
useless. And, its use doesn't even comply to PEP 337 ("py."
prefix). So if a student wants to implement PEP 337 in SoC,
he/she is welcome to do this consistently, and any obscure
logging bugs will certainly show up soon after that.

----------------------------------------------------------------------

Comment By: Jim Jewett (jimjjewett)
Date: 2006-05-17 21:34

Message:
Logged In: YES 
user_id=764593

(1)  I don't think logging should be removed from the 
stdlib.  At the very least, the reasoning should be added 
to PEP 337, which says to *add* logging to the standard 
library.  http://www.python.org/dev/peps/pep-0337/  (There 
will probably be a Summer Of Code student funded to do 
this; if it is a problem, lets fix the problem in the 
logging module.)

(2)  Logging isn't really as unstable as you seem to think 
Beta implies; it is probably more stable than the newer 
cookielib, let alone the combination of cookielib, urllib2, 
and Processors.  (John Lee has been making long-overdue 
fixes to urllib2 -- and processors in particular -- because 
he was the first to really understand it well enough; these 
fixes are generally triggered by immediate problems and may 
not be complete fixes.)

I will agree that it might make sense to remove the beta 
marker from the version of logging that is distributed in 
the stdlib.

(3)  What else was shipped with those applications which 
caused this?  Which version of logging did you have?

Both tracebacks could be caused if the root logger were not 
a normal logger (and its manager therefore not a normal 
manager).  Vinay has taken some steps to allow 3rd party 
libraries to override the class of even the root logger, 
but doing it *right* is fairly subtle.

Another possibility is that you got burned by threads 
allowing access to half-constructed loggers or managers, or 
by broken PlaceHolders/fixups in the manager.  Again, this 
can't happen unless someone is doing at least two dangerous 
things, but ... it has triggered a few of the changelog 
entries.




----------------------------------------------------------------------

Comment By: Georg Brandl (gbrandl)
Date: 2006-05-17 14:46

Message:
Logged In: YES 
user_id=849994

Resolved with rev. 46027 by introducing a global "debug"
flag, like other libraries do.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=1484758&group_id=5470