Hi Darren,
Thank you for your review. I added code to address both your suggestion
below. Please see my updated webrev at:
https://cr.opensolaris.org/action/browse/caiman/ktung/engine-bugs-updated/webrev/
Thanks,
--Karen
On 07/14/11 02:44, Darren Kenny wrote:
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
