[Python-Dev] PEP 282 Implementation

[Vinay Sajip]

> In general the code looks good.  Only one style nits: I prefer
> docstrings that have a one-line summary, then a blank line, and then a
> longer description.

I will update the docstrings as per your feedback.

> There's a lot of code there!  Should it perhaps be broken up into
> different modules?  Perhaps it should become a logging *package* with
> submodules that define the various filters and handlers.

How strongly do you feel about this? I did think about doing this and in
fact the first implementation of the module was as a package. I found this a
little more cumbersome than the single-file solution, and reimplemented as
logging.py. The module is a little on the large side but the single-file
organization makes it a little easier to use.

> - Why does the FileHandler open the file with mode "a+" (and later
>   with "w+")?  The "+" makes the file readable, but I see no reason to
>   read it.  Am I missing?

No, you're right - using "a" and "w" should work. I'll change the code to
lose the "+".

> - setRollover(): the explanation isn't 100% clear.  I *think* that you
>   always write to "app.log", and when that's full, you rename it to
>   app.log.1, and app.log.1 gets renamed to app.log.2, and so on, and
>   then you start writing to a new app.log, right?

Yes. The original implementation was different - it just closed the current
file and opened a new file app.log.n. The current implementation is slightly
slower due to the need to rename several files, but the user can tell more
easily which the latest log file is. I will update the setRollover()
docstring to indicate more clearly how it works; I'm assuming that the
current algorithm is deemed good enough.

> - class SocketHandler: why set yourself up for buffer overflow by
>   using only 2 bytes for the packet size?  You can use the struct
>   module to encode/decode this, BTW.  I also wonder what the
>   application for this is, BTW.

I agree about the 2-byte limit. I can change it to use struct and an integer
length. The application for encoding the length is simply to allow a
socket-based server to handle multiple events sent by SocketHandler, in the
event that the connection is kept open as long as possible and not shut down
after every event.

>   - method send(): in Python 2.2 and later, you can use the sendall()
>     socket method which takes care of this loop for you.

OK. I can update the code to use this in the case of 2.2 and later.

> - class DatagramHandler, method send(): I don't think UDP handles
>   fragmented packets very well -- if you have to break the packet up,
>   there's no guarantee that the receiver will see the parts in order
>   (or even all of them).

You're absolutely right - I wasn't thinking clearly enough about how UDP
actually works. I will replace the loop with a single sendto() call.

> - fileConfig(): Is there documentation for the configuration file?
>

There is some documentation in the python_logging.html file which is part of
the distribution and also on the Web at
 http://www.red-dove.com/python_logging.html - it's in the form of comments
in an annotated logconf.ini. I have not polished the documentation in this
area as I'm not sure how much of the configuration stuff should be in the
logging module itself. Feedback I've had indicates that at least some people
object moderately strongly to having a particular configuration design
forced on them. I'd appreciate views on this.

Many thanks for the feedback,


Vinay Sajip