-
Notifications
You must be signed in to change notification settings - Fork 43
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Log a notice every 100 puppet resources completed #306
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be cleared to the user:
2020-11-05 20:29:03 [NOTICE] [configure] Starting system configuration. The total number of configuration tasks may increase during the run. Observe logs or specify --log-level to see individual configuration tasks.
2020-11-05 20:34:23 [NOTICE] [configure] 100 configuration tasks out of at least 1322 complete.
This looks like we're inventing a progress bar again. |
Adding some notion of progress for the user when running this long running step. That feels like a good thing. I can not tell if your comment is positive or negative. |
423e202
to
06278be
Compare
With your requests @wbclark this is what I have now:
|
[test kafo] |
Failures caused by theforeman/foreman-infra#1515 |
[test kafo] |
06278be
to
a841544
Compare
I opened #309 to play with renaming That shouldn't block this, and I can rebase that one once this is merged, but I'm mentioning it here because this PR is what made me notice the somewhat confusing name of that option. |
[test kafo] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good amount of information to give to the user by default.
It includes:
-
Very high level overview of stages of the installer run, without going into too much detail by default
-
Some indication of progress in the long running puppet apply stage without setting any false expectations or over-hiding information
-
Some information about how the use can change the behavior of the terminal output if they desire more information.
Here are some potential gaps I notice with the complete verbose output as it stands currently, which we may want to address in future PRs:
-
We could print a message stating the path to the complete logs. This is good knowledge for all users of the installer to have, and therefore good information to find when using the installer for the first time.
-
Perhaps some instructions on changing log levels and output behavior are appropriate; for example,
Running installer in log output mode with log level NOTICE. You can change the terminal output log level with -l, or use --progress-bar to display a simple progress bar instead of log based output.
The installer does this through hooks. I agree it could be a feature of Kafo instead of relying on the scenario to implement. |
With verbose output and the baseline notice level, when the installer runs the
puppet apply
stage there is a long period of time coupled with no output that can give the impression the installer is hung. This changes opts to output to the user every 100 resources that have completed evaluation to give a sense of progress.Example output:
I intended to include a sense of the total, that is
X out of Y
but found that the total value can actually change as theapply
progresses. But maybe the total, even though it changes, is nice because it gives a sense of how close to being done things are?Here is example output of how that would look (and how the total changes):