On 01/05/10 08:40, Keith Mitchell wrote: > >>> Hi Sarah, >>> >>> Thank you for the review. I'll answer your comments on the profile >>> code here (including the comments from your other response). >>> >>> - Keith >>> >>>>> 429 logging.debug("Create tgt.Disk - geometry.blocksz = " >>>>> + str(g.blocksz)) >>>>> 430 logging.debug("Create tgt.Disk - geometry.cylsz = " + >>>>> str(g.cylsz)) >>>>> 431 logging.debug("Create tgt.Disk - name = " + name) >>>>> 432 logging.debug("Create tgt.Disk - blocks = " + >>>>> str(blocks)) >>>>> 433 logging.debug("Create tgt.Disk - controller = " + >>>>> str(controller)) >>>>> 434 logging.debug("Create tgt.Disk - boot = " + str(boot)) >>>>> 435 logging.debug("Create tgt.Disk - removable = " + >>>>> str(removable)) >>>>> 436 logging.debug("Create tgt.Disk - vendor = " + >>>>> str(vendor)) >>>>> 437 logging.debug("Create tgt.Disk - serialno = " + >>>>> str(serialno)) >>>>> 438 logging.debug("Create tgt.Disk - use_whole = " + >>>>> str(use_whole)) >>>>> 439 logging.debug("Create tgt.Disk - fdisk = " + str(fdisk)) >>>>> 440 logging.debug("Create tgt.Disk - vtoc = " + str(vtoc)) >>>>> 441 logging.debug("Create tgt.Disk - child_list_size = " + >>>>> 442 str(len(child_list))) >>>> >>>> The debug logging would be better put in a separate utility python >>>> module which you call. >>> >>> I'll check with Karen, but I think these logging statements are >>> mostly temporary. >> >> I think that having debug logging, or debug code in from the start is >> fine, I just think it should be based on something the user sets to >> trigger the debug action. > > > The python logging module won't add those statements to the log, > unless the user has started the installer with a "-v debug" flag. By > default, we plan on only having logging.info, warning, error and > exception level messages in the log (with 'info' roughly tracking the > user's selections through the UI). > > We do plan on refining our use of the logging module - cleaning up > excessive logging, formatting the strings to improve performance, > standardizing the logging levels for consistency (i.e., making sure > the verbosity matches the intent of each logging statement).
Hi Sarah, I want to respond to your comment above. Keith is taking care of other comments in the "profile" code. These debugging statements are very helpful when we need to figure out target initiation failures. I do agree with you that having them in a separate module is better. The best way to do it is actually to define __str__() functions for all the tgt objects, then, we can just call logging.debug() once passing in the tgt object, and everything will get printed as needed. Since we might or might not have enough time to do it before we do the final putback, I have moved all these debugging statements to a separate module. When the __str__() functions are implemented for the tgt objects. They will be removed, and the __str__ methods will be used instead. Thanks, --Karen