Hi Keith - Thanks for the review. See my responses below. ginnie
On 03/23/10 12:41, Keith Mitchell wrote: > Hi Ginnie, > > I think the document is looking fantastic. The detail has helped me > understand the logging module. > > My remaining concern is with the API for init_logger and add_handler. > > I had to read ahead all the way down to use cases to fully understand > the "handler" argument. It appears that a string or constant will be > mapped to a handler type by the init_logger call, and instantiated > using args also passed into init_logger. This explains Sarah's desire > to have arbitrary arguments passed in, but even with that, it sounds > overly restrictive. Why not instantiate the handler, and then pass in > the handler reference to init_logger? > > e.g. > handler = logging.FileHandler(...) > logging.init_logger(app_name, handler, logging.INFO) > In this way, any logging handler, even handlers which are not directly > defined in the logging module (e.g., an application specific subclass) > can be passed in, and the init_logger function doesn't have to worry > about understanding those things. The application won't be calling anything with "logging.". The logging interaction is going to be managed directly by the logging module. If a handler isn't defined in the logging module, it couldn't be used for logging. The init_logger function gives the application an entrance to the logging module. It doesn't have direct access to it. From my understanding of how this would be implemented, the application would make a request to the engine for logging, which the engine passes on to the logging module. I don't have a problem with the application calling for a handler first, before initializing the logger, but there is overhead either way. The application would do the following: handler = add_handler( file) init_logger(app_name, handler, logging.INFO) However, my plan was to allow an application to initialze w/o requiring a handler object, and have a default set up if nothing is provided (this would be to a file handler with a agreed upon location for the file). init_logger(app_name) That would be about as simple as it could get. > > With intelligent defaults for keyword arguments for handlers, this can > even be reduced further to something as simple as: > > logging.init_logger(app_name, logging.Filehandler()). > > Similarly, with init_handler - why not call it "add_handler"? I'm fine with changing the name. That's not a problem. > Let the consumer pass in a constructed handler, and have the > add_handler code handle any additional registration, such as setting > up the proper formatting, logging level, and the like. Done properly, > this would be far more flexible, yet maintain simplicity for the > default cases. If defaults for kwargs is not sufficient, provide a > 'generator' or 'factory' function for those specific classes. For > example, if the HTTP defaults require logging module context, then a > function such as: > logging.generate_http_handler(host, url, ...) > > If the argument is complexity - I argue that it's more complex to ask > the consumers to construct the **kwargs object, instead of the more > natural method of simply instantiating the desired handler instance. The problem with this approach, if I'm understanding you correctly, is that the consumers aren't directly calling logging module interfaces. The idea is to use the "wrapper functions", which pass the info to the engine, and then, have the engine interact directly with the logging module. I don't think that what you are proposing really shifts the complexity; it just moves the responsibility from the logger to the application. The generate_http_handler function would accomplish the same thing as init_handler(http, hostname, url) or add_handler(.....), if I change the name. One way or another the information has to be propagated to the logger. If I'm not understanding you, please let me know. > > Use cases: > > Pseudo code for log_messages: I know it's just sample code, but, if > log levels mapped to integers in the same way Python's logging module > does, then it's a much simpler task to do something like: > > def log_message(...): > for handler in self.handlers: > handler.log(self.logging_level, message, ...) The logging levels that exist in the Python logging module will be used. If we need to sub-class additional ones (which we will do for progress reporting), then we'll add other log levels. The way the Python logging module works is when you set up a handle with a default logging level, then anytime a message with that logging level is generated, it will be published to that handler. So, when the application generates log_message( level, message), it will go to the correct handlers that have been set up to receive messages at that level. I haven't looked specifically at how the Python logging module does this, but I wouldn't be surprised if it is implemented similarly to what you've done. Let me know if you have any other questions. Thanks again for your comments. ginnie > > Thanks, > Keith > > On 03/12/10 04:39 PM, Virginia Wray wrote: >> Hi - >> >> I've posted the second draft of the Logging design doc at: >> >> http://hub.opensolaris.org/bin/view/Project+caiman/Logging >> >> Select Loggingv1.2.pdf >> >> Please provide feedback by March 24, 2010. >> >> thanks, >> -- Ginnie
