Hi Geoffrey,
A couple nits on the changes:
- Can you add a comment on the changes you made
indicating what that code is doing.
- You have 60 hard coded everywhere. I think it
will be useful to define it as a constant, so, if we feel
that reporting progress every 60 seconds is too much/little,
we can easily tune the value.
Can you update the bug to say what the fix is?
In the Evaluation section, you mentioned that you will add
logging "periodically" to inform the progress. In the comment
section, you show the sample output of reporting progress
every minute. From reviewing the code, I can see that your update
is to report progress every minute. Can you clearly spell that out
in the bug report?
Thanks,
--Karen
On 05/31/12 13:03, Geoffrey Hart wrote:
Afternoon all!
Could I please have a code review for:
BUG: 7171973 IPS transfer code should show more logging during install
of packages
CODE REVIEW:
https://cr.opensolaris.org/action/browse/caiman/ghart/IPS_Install_Progress/
DESCRIPTION: Adding progress logging during the install of IPS
packages so during the 10+ minutes of install time there are now log
entries to show the install is proceeding and not hung.
OLD LOG ENTRY:
2012-05-29 16:19:30,799 InstallationLogger.generated-transfer-1653-1
DEBUG Install Phase ... Started.
2012-05-29 16:31:40,035 InstallationLogger.generated-transfer-1653-1
DEBUG Install Phase ... Done.
NEW LOG ENTRY:
2012-05-29 16:19:30,799 InstallationLogger.generated-transfer-1653-1
DEBUG Install Phase ... Started.
2012-05-29 16:20:30,803 InstallationLogger.generated-transfer-1653-1
DEBUG 24% complete.
2012-05-29 16:21:30,804 InstallationLogger.generated-transfer-1653-1
DEBUG 32% complete.
2012-05-29 16:22:32,195 InstallationLogger.generated-transfer-1653-1
DEBUG 40% complete.
2012-05-29 16:23:32,239 InstallationLogger.generated-transfer-1653-1
DEBUG 47% complete.
2012-05-29 16:24:32,241 InstallationLogger.generated-transfer-1653-1
DEBUG 55% complete.
2012-05-29 16:25:32,264 InstallationLogger.generated-transfer-1653-1
DEBUG 61% complete.
2012-05-29 16:26:32,347 InstallationLogger.generated-transfer-1653-1
DEBUG 64% complete.
2012-05-29 16:27:32,365 InstallationLogger.generated-transfer-1653-1
DEBUG 72% complete.
2012-05-29 16:28:34,229 InstallationLogger.generated-transfer-1653-1
DEBUG 75% complete.
2012-05-29 16:29:34,291 InstallationLogger.generated-transfer-1653-1
DEBUG 81% complete.
2012-05-29 16:30:36,185 InstallationLogger.generated-transfer-1653-1
DEBUG 85% complete.
2012-05-29 16:31:36,189 InstallationLogger.generated-transfer-1653-1
DEBUG 98% complete.
2012-05-29 16:31:40,035 InstallationLogger.generated-transfer-1653-1
DEBUG Install Phase ... Done.
TESTING:
- Confirmed that new logging works and appears in this new format.
- Verified that the install still succeeds and the OS boots correctly
after install
Thanks,
Geoffrey
_______________________________________________
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
