-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Don't print prelude in terminal mode #3890
Conversation
We can't correctly print simple messages for prelude events when doing progress based display in a terminal, as it would lead to resetting the display of the table rendering. This does mean that `--show-config` no longer works in the default terminal display mode - but it's not clear it *can* work correctly (at least as currently implemented) since it doesn't cleanly participate in the table rendering. For cases where `--show-config` is not set (the norm) -nothing would have been printed anyway, so the changes here just avoid resetting the table rendering unnecessarily. Fixes #3469.
pkg/backend/display/progress.go
Outdated
payload := event.Payload.(engine.PreludeEventPayload) | ||
display.writeSimpleMessage(renderPreludeEvent(payload, display.opts)) | ||
if !display.isTerminal { | ||
// If we are in a terminal, then we may have already rendered progress events, and so we |
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.
another option is to forward this as an ephemeral message to the root resource. so it still shows up somewhere, but doesn't impact the display.
But this is a reasonable place to start.
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.
another option is to forward this as an ephemeral message to the root resource. so it still shows up somewhere, but doesn't impact the display.
Yeah, this is what I'd expect us to do
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.
if this contains important data, we could also treat this like we do summary-data, and put it at the end. It's unclear what information in this is important though.
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'm honestly not sure anyone knows that --show-config
exists - and without it set this does nothing (except introduce the bug we are fixing here).
But when a user explicitly passes --show-config
we render something like this today:
Previewing update (dev):
Type Name Plan Info
pulumi:pulumi:Stack perf-dev warning: resource plugin aws is expected to have version >=1.20.0, but has 1.19.0-alpha.1579202679+gd1b5c8a47; the wrong versi
Configuration:
aws:region: us-east-1
Type Name Plan Info
pulumi:pulumi:Stack perf-dev 2 warnings
Diagnostics:
pulumi:pulumi:Stack (perf-dev):
warning: resource plugin aws is expected to have version >=1.20.0, but has 1.19.0-alpha.1579202679+gd1b5c8a47; the wrong version may be on your path, or this may be a bug in the plugin
warning: resource plugin aws is expected to have version >=1.20.0, but has 1.19.0-alpha.1579202679+gd1b5c8a47; the wrong version may be on your path, or this may be a bug in the plugin
These two lines are the key piece of that:
Configuration:
aws:region: us-east-1
We could show that at the end - though I think it is really more of an input thing which is intended to be shown at the beginning. It's just that we don't get the data early enough to actually show it before beginning to render the table.
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.
Going to update so that it prints this
$ /opt/pulumi/bin/pulumi up --show-config
Previewing update (dev):
Type Name Plan Info
pulumi:pulumi:Stack perf-dev 1 message
Diagnostics:
pulumi:pulumi:Stack (perf-dev):
Configuration:
aws:region: us-east-1
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.
👍 Def prefer this way.
We can't correctly print simple messages for prelude events when doing progress based display in a terminal, as it would lead to resetting the display of the table rendering.
This does mean that
--show-config
no longer works in the default terminal display mode - but it's not clear it can work correctly (at least as currently implemented) since it doesn't cleanly participate in the table rendering.For cases where
--show-config
is not set (the norm) - nothing would have been printed anyway, so the changes here just avoid resetting the table rendering unnecessarily.Fixes #3469.