Thanks for your response. I'm fine with you pushing. Jean
Karen Tung wrote: > Hi Jean, > > Thank you for the code review, please see my response inline. > > Jean McCormack wrote: >> Here's my comments: >> >> General: what if dc_log = logging.getLogger(DC_LOGGER_NAME) returns null >> or generates an exception? >> >> > It will never return null or generate an exception. The way the > Python logging works > is that if DC_LOGGER_NAME doesn't already exist for some reason, it will > create a new one for you. If it already exist, it will return you a > handle to that instance. > If it really doesn't exist, the new one created will use the default > logging settings. > Because of this, I can get away with not having to pass the handle > around, and not > checking the return value, because it will never be null. In fact, in > some place that I read, > it says that they make it behave this way so we don't need to pass the > handle around. > >> DC_checkpoint.py: >> line 280: Shouldn't that be if (log_handler != None): >> > Yes, it should be. Fixed. >> Question: Should all print statements now go to dc_log? Or >> only select ones? >> > I carefully examined all the functions in DC_checkpoint.py. All the ones > that are called after the DC log is initialized will go to the > dc_log. All the > ones that are called before the DC log is initialized just go to the > screen. > The DC logging can't be setup until the work area is setup. So, all > the functions > that involve creating, checking the work area and stuff won't be able to > do logging to the DC log. >> finalizer_checkpoint.py: >> In other code you modified the shell_cmd calls to include logging. >> Do you want to >> do that here too? >> > Yes, good idea. Added. >> finalizer_rollback.py: >> ditto >> > Added as well. >> line 66: that should be 9. One of the "length" is due to the >> program itself, not an arg. >> >> >> > Fixed. >> post_bootroot_pkg_image_mod: >> Please add the usr zlib compression algorithm to the header comments >> > Added comment to header >> finalizer.py: >> line 609: Should there be a space after signal and before " ? >> > Yes, space added. >> transfer_mod.py: The assumption here is that you won't be logging via >> the liblogsvc module >> and via this logging module. Correct? >> > If a logging handler is passed in, then, the logging handler will be > used for logging. > If no logging handler is passed in, then, everything will be done as > before, which is > using the liblogsvc module. > > Thanks again for your comments. > > --Karen >> Jean >> >> >> >> Karen Tung wrote: >> >>> Hi, >>> >>> Please do a code review for changes to fix the following bugs: >>> >>> 3341 stop_on_error doesn't seem to work for finalizer scripts >>> 3354 Logging of the DC app >>> 3482 Return status from DC needs to reflect errors encountered >>> 3417 /var fails pkg verify >>> 3536 Change constructor to use lzma for non-usr archives on primary >>> live CD >>> 3460 Remove attrs and tags in <packages> section in manifest >>> >>> webrev: >>> >>> http://cr.opensolaris.org/~ktung/dc_logging/ >>> >>> Thanks, >>> >>> --Karen >>> _______________________________________________ >>> caiman-discuss mailing list >>> caiman-discuss at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >>> >> >> _______________________________________________ >> caiman-discuss mailing list >> caiman-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss >> >
