Other than what Drew has commented on, the only questions/comments if have are:
install_engine/__init__.py: - Rather than using "name + InstallEngine.CP_COMPLETED_SUFFIX" at different points in the code, I wonder would it be better to do like you have for other things and provide a "get_completed_name" (or similar) method which does this. - lines 1239+ _get_doc_snapshots() should probably handle the case where root_dir is None. Thanks, Darren. On 13/07/2011 23:14, Drew Fisher wrote: > Karen, > > usr/src/lib/install_engine/__init__.py > -------------------------------------- > > 627: use if not doc_list > 630: use if not self._checkpoints > 869, 877, 884, 888: use if cp_data is not None > 1136: maybe add "necessary" to the end of the message? > > "First checkpoint is different than previous first checkpoint, no rollback > necessary" > > 1138: remove the spaces inside the brackets: > > resumable_cp_list[index-1] > > 1250-1259: 'file' is a reserved word in Python. Can we use something else > here? > > 1257-1259: why not use a one-liner here? > result = [f[1] for f in file_list_with_time] > > > Otherwise this looks great! Thanks for fixing these! > > -Drew > > > > > On 7/13/11 3:40 PM, Karen Tung wrote: >> Hi, >> >> I would like to get a review of my changes to fix the following 2 bugs: >> 7004000 <http://monaco.us.oracle.com/detail.jsf?cr=7004000> Install engine >> enhancement to save original args/kwargs list before calling checkpoint's >> execute >> 7011388 <http://monaco.us.oracle.com/detail.jsf?cr=7011388> resuming from >> failed checkpoints does not work >> webrev: >> https://cr.opensolaris.org/action/browse/caiman/ktung/engine_bugs/webrev/ >> >> Testing: >> >> - I made sure that all the install engine and DC unit tests are run cleanly. >> - I also manually tested various pause/resume DC execution scenarios and >> made sure they all work as expected. >> >> Thanks, >> >> --Karen >> >> >> _______________________________________________ >> caiman-discuss mailing list >> [email protected] >> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss > > > _______________________________________________ > caiman-discuss mailing list > [email protected] > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss _______________________________________________ caiman-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
