[Python-Dev] PEP282 and the warnings framework

Walter Dörwald walter@livinglogic.de
Fri, 17 May 2002 13:39:29 +0200


Vinay Sajip wrote:

>>>The last part (about __str__) is reasonable. But I disagree with the
>>>statement that the LogRecord should do formatting.
>>
>>I meant only the formatting of the message itself, i.e. the
>>"self.msg % self.args" part. Timestamps, file/line info etc. should
>>be the responsiblity of the Logger/Formatter.
> 
> Oh, sorry I misunderstood. I thought you meant that the self.msg % self.args
> was just the *default* implementation, and that subclasses might change
> this.
> 
>>You're right, configuration should be changable at runtime. Doing this
>>when configuration involves custom classes is hard, but this shouldn't
>>be your job, but the job of those that implement these custom classes.
> 
> I agree it's not my job. I only point out that it involves extra work,
> whoever ends up doing it. I don't want people who want to use logging in a
> simple way (for less complex applications) to have to do the extra work.

They don't have to as long as they don't use custom LogRecord.

 > [...]
> Perhaps I used the wrong term? According to my reading of "Design Patterns"
> (Gamma, Helms et al) the term "factory method" is intended to refer to a
> method which encapsulates creation of an object. It's intended to be
> redefined in subclasses, so that they can manage their own object creation.

Exactly, but then have have to same problem one level up.

> But I get what you mean.

I just did't find setLoggerClass().

>>... which is done here. But this would change the LogRecord classes
>>that are used on a global scale. I'd like to decide which class should
>>be used when I'm creating the event. BTW, in setLoggerClass() you should
>>probably use
>>     if not issubclass(klass, Logger):
>>instead of
>>     if not (Logger in klass.__bases__):
> 
> 
> Thanks for the tip. I've updated the source to use issubclass for 0.4.5!
> 
>>What I want is something like this (similar to what the warning
>>framework does):
>>
>>def log(event, eventtype=LogRecord):
>>     if not isinstance(event, LogRecord):
>>         event = eventtype(event)
>>     ... the rest of the code
> 
> 
> I think a better design is to pass in a callable which constructs the
> LogRecord or a derived class thereof. This way, you can use a class, a
> function, a bound method which has access to other context, or whatever.

This will work in my version without a problem, because
a function is not an instance of LogRecord.

> def log(..., record=LogRecord):
>     ...check to see if logging will happen...
>     record = record(...args)
>     self.handle(record)

OK, but when you pass in something that is already a LogRecord instance
you have a problem with the "record = record(...args)" call. And
maybe the LogRecord instance would like to decide about whether
logging should happen or not.

> The callable would be called with the same arguments as Logger.makeRecord
> currently is - it can do what it wants with them.

But why do you want to move the construction of the LogRecord
instance into the logger code. Instead of passing the constructor
arguments of the LogRecord and the type/callable to the logger
and let the logger create the LogRecord instance, why shouldn't
it be  possible to pass in a LogRecord instance?

> So you could do...
> 
> def myRecordMaker(**kwargs):
>     record = MySpecialRecord(...)
>     record.__dict__.update(kwargs)
>     return record
> 
> and then:
> 
> logger.warn("Hello, %s", "world!", record=myRecordMaker)

So go one step further and allow:
   logger.warn(myRecordMaker("Hello, %s", "world!"))

(I assume that MySpecialRecord is derived from LogRecord).
Either the LogRecord constructor should be responsible
for creating the timestamp and finding file/line info. etc.
or this could be done by the logger (via calling a method
on the LogRecord instance).

> I think this is neater and more Pythonic than the factory method :-) Let me
> mull this one over a bit to see if there are any other repercussions.
> Comments, please?

Bye,
    Walter Dörwald