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
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.
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...
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
