On 06/05/12 01:00, Darren Kenny wrote:
Hi Martin,
The bug 7097012 does present something to be handled here I suppose -
if the installer
is being run from a TTY - like DC often is, but also if you run
auto-install from a
logged in shell (not a common use case I would think).
In the bug 7097012, it enabled the 'show_output=true' flag being passed
down to
the IPS transfer, which basically enables in-line (via ^H type output)
showing
of progress like:
Installing Package: 99/9999
With 7097012, setting 'show_stdout=true' for the global zone is what
enables the line-by-line output we see on the console which comes from
the InstallCLIProgressTracker().
or similar.
But honestly this was a simple fix that maybe should be disabled now,
with this fix
which is doing something similar - at least the show_stdout part which
generates the
above output.
The changes being made by Geoff here actually enhance what's seen in the
log *and* the console for the InstallCLIProgressTracker() class. Notice
that InstallCLIProgressTracker() has the _logger_output() method which
wraps around the actual call to debug() or info() based on 'show_stdout'.
Geoffrey, can you check how your fix handles this by running DC, or AI,
in a tty? I
suspect it will cause some weird interspersed output...
When running in a tty (which you can check for AI by installing a zone),
the InstallFancyProgressTracker() is used, and that class logs to
debug() only. So the changes being made here won't affect what's seen
in the console when run on a tty.
-ethan
Thanks,
Darren.
On Fri Jun 1 23:29:06 2012, Martin Widjaja wrote:
Geoff,
As for as displaying to the console, very good point. I will correct that to
show status to the console.
In case you'd like to see, the other parts of progress report (other than IPS)
are now printed to the console (see bug 7097012), just FYI in case you were
wondering and need to sync up the output format.
Thanks,
Martin
On 6/1/2012 2:56 PM, Geoffrey Hart wrote:
Yes we have everything downloaded already. I didn;t add anything for progress
on the download phase since it already shows the progress for download on the
console. The motivation for this fix is the 10 or so minutes it takes for
install where we do no logging to the console or log file.
As for as displaying to the console, very good point. I will correct that to
show status to the console.
So with this in mind, what are your thoughts on logging every minute vs logging
after every 5 or 10 %?
Thanks,
Geoffrey
On 06/01/12 10:25, Darren Kenny wrote:
On Fri Jun 1 17:11:44 2012, Geoffrey Hart wrote:
Thanks Darren.
I will run this through pep8 as I make the other corrections you mentioned.
I selected to log every minute rather than every 5% so there would be
regular log entries. With network problems, it could be a long delay
between entries if I logged every 5% of the way. Your thoughts?
I can see where you're coming from, but at this point - install - we've
already downloaded everything, haven't we?
Or are you adding the progress at the download phase too - I'm not
totally
sure of of the phases based on the names in the webrev...
BTW, does any of this appear on the console during an AI install?
Ideally we want it to, so people will see progress without logging in,
and I'm thinking that it would need to be at the INFO
level rather than DEBUG level for that to be the case.
Thanks,
Darren.
- Geoffrey
On 06/01/12 04:39, Darren Kenny wrote:
Hi Geoffrey,
Thanks for looking at this...
In general I'm happy with the approach, but have some minor comments in
addition to Karen's:
- You could probably omit the need for the percent_complete variable
totally, if you changed the lines to look like:
if time_diff.seconds>= 60:
self._logger_output("%s%% complete." %
(100 * self.act_cur_nactions) / self.act_goal_nactions)
- Another approach, rather than using time, would have been to just
remember the last percentage calculated, and just report every 5%,
or similar.
Either works for me - and in someways 60 seconds is better, just
wondering if you considered this approach and ruled it out?
- The indentation around the percentage calculation is confusing, and
possible not PEP8 clean - have you run PEP8 against the code?
An example of this is:
if time_diff.seconds>= 60:
percent_complete = (100 *
self.act_cur_nactions)/self.act_goal_nactions
self._logger_output("%s%% complete." % percent_complete)
this would be more readable, if calculation was indented differently,
(and also PEP8 cleaner),
if time_diff.seconds>= 60:
percent_complete = (100 *
self.act_cur_nactions) / self.act_goal_nactions
self._logger_output("%s%% complete." % percent_complete)
or even:
if time_diff.seconds>= 60:
percent_complete = \
(100 * self.act_cur_nactions) / self.act_goal_nactions
self._logger_output("%s%% complete." % percent_complete)
My preference would be the latter...
Thanks,
Darren.
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
_______________________________________________
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
