Hi Dan, Thanks for taking the time to discuss this :)
On 15/06/2012 22:34, Dan Price wrote: > > Greetings, > > I apologize for the rush, but would appreciate comments as soon as > possible. Hopefully this should not be lengthy, as the changes are > simple to review. > > In the very near future I will be integrating changes into the IPS gate > which substantially improve and rework the progress tracking we do in the > packaging system. In the course of discussing these changes with Ethan he > suggested that I should have the AI team review the output changes I am > making. So that is part 1 of this request. > > This bugfix also represents an API boundary; we need to make > corresponding changes in Caiman in order to cope with the new API. > That is part 2 of this request. > > Part 1: Output Changes > ---------------------- > Caiman will no longer carry around its own subclasses of the progress > trackers. It will simply hook up existing progress trackers as needed, > and the packaging system will do the rest. The new code provides a > superior user experience for the CLI progress tracker, including: > > - visibility into the different phases of planning. > - visibility into manifest fetching. > - monitoring of download phase percentage progress, > megabytes downloaded, and an estimate of download speed. > - monitoring of action execution. > - information at the end of each phase about how long the > phase took. > Just to be clear, and maybe this is Part 2, but the way that this is being changed is that the logging will now be done in the pkg code via the logger, is that right, thus removing the need for us to do the sub-classing we've had to do before, is that correct? > The item that Ethan thought most significant is that we no longer print > the name of each package as it is downloaded. I understand that some > admins may wish to see this information, and so I have modified the AI > code so that, at the DEBUG log level, it prints the installation plan to > the *log* (not to the screen) before commencing the download. This is > similar to the way you can get the pkg plan by passing -v to pkg > subcommands, although I simplified the output format (it's all of 3 > lines of code). > > [As an aside; in the future we may not download packages one-at-a-time, > so I would discourage this anyway] While there have been many requests from admins for us to provide progress about what package is being installed, etc. I feel that the real desire for most people is to have a meaningful progress output, so that they know things haven't just hung, or stopped silently. > > Ethan encouraged me to post the output printed to the console when > performing a bare-metal install. Keep in mind that the periodic output > you see below is currently dumped every 20 seconds (which you can very > by looking at the timestamps); however, that's a configurable parameter, > and I'm happy to tune it to whatever you prefer-- I personally felt that > 3 times a minute was the outer limit to "keep the admin warm."-- a > nice balance between not spamming the terminal and not freaking out > the user. Can we be clear about what you mean when you say 'console' here? I'd like to confirm that it's not just the tty, since we also redirect some output to /dev/sysmsg when the user is not logged on the console, which is the norm in AI's case. This is in comparison to what happens when you are user doing a zones install, where the output is to the user's tty. Can I assume that the correct thing happens in both of these cases? It looks like you have take this into account in the use_stdout toggle, but just want to be sure... > > As before, output will also go to the log. Ethan and I worked together > to ensure that nothing changes except for the format of the messages. That's great. > > The output from doing a bare-metal install is attached as a text file > to this message. The corresponding log file is also attached; look for > the string "Installation Plan" to see the new output. I have gzipped it, > as it is large. > > The output for installing a zone or when using DC uses the fancy progress > tracker, just as before, and is largely the same as before. It has > been cleaned up and is much richer. There's nothing there to review. To be honest, it all sounds good to me... :) > > Part 2: Code Changes > -------------------- > Code which implements the changes is posted here: > > https://cr.opensolaris.org/action/browse/pkg/dp/pkg-progress-ai/ > > The 15 or so lines of replacement code is annoyingly complicated-- the > dance of loglevels is a pain-- but I believe it mirrors exactly the way > the code worked before my putback, and of course I am testing it. I would > encourage the install team to fix this code in the future-- it should > not be the case that "sometimes a message is DEBUG and sometimes it is > INFO," but that's another wad for another day. Our preference is that as much information as possible should always be logged to the install_log file, which the tty or sysmsg should see enough to know that things are happening. As such, I think that the level people seem to want in the installer on the tty or sysmsg is generally with more detail than most people would desire if running pkg at the command-line - equivalent to 'pkg -v' usually - and it would appear that to be able to get this level output we need to set the pkg logging level to the INFO level to achieve this. In the case where we can do the fancy logging (i.e. we have a tty, and we can use things like ^H, ^U, etc to make things look nicer) we need to ensure the logging is still done to the install_log, while the INFO level is now being done using the fancy output mechanism as well. As such, I don't feel that this is that much of a hack, but is instead working within the boundaries of the API that is being provided by the pkg progress trackers to achieve the needs we have in the installer. > > The progress tracking wad is posted below as reference materials; I am not > requesting comment on that work in this thread; that code is still > undergoing minor changes in response to codereview feedback. > > https://cr.opensolaris.org/action/browse/pkg/dp/pkg-progress/ > > Thanks for your attention and help. Thanks especially to Ethan, > without whom I would be sunk. In general the changes in the AI code look fine by me. Thanks, Darren. > > -dp > > > > _______________________________________________ > 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
