On 01/06/2012 22:56, 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.
>
OK, thanks, just wanted to be sure... :)
> As for as displaying to the console, very good point. I will correct
> that to show status to the console.
Thanks.
>
> So with this in mind, what are your thoughts on logging every minute vs
> logging after every 5 or 10 %?
Every minute is fine - so even if there is some long install task (which I
hope there isn't anything longer than 1 minute TBH) it would still show
some output so the user doesn't think things have stalled...
Thanks,
Darren.
>
> 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
