> 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
RetroSearch is an open source project built by @garambo | Open a GitHub Issue
Search and Browse the WWW like it's 1997 | Search results from DuckDuckGo
HTML:
3.2
| Encoding:
UTF-8
| Version:
0.7.4