Skip to content
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

add output for running policy packs #14493

Merged
merged 8 commits into from Nov 15, 2023
Merged

Conversation

tgummerer
Copy link
Collaborator

Description

🚧

This is a work in progress attempting to add some output for when policy packs are being loaded/compiled (which I realized was the actual reason for the slowdown, not that they were being run already). There's still some pretty obvious code quality issues as I've been throwing this together, and the URN's are not constructed properly yet either, so some of the output is less helpful than it could be. I'm mainly opening this to get some early feedback on whether this is the appropriate place to add the output, or if there are better alternatives.

Also including a gif here of what this looks like. In particular the Name here needs to be improved, and the "Plan" might want to say "loading/compiling" or something similar instead?

policy-pack

Fixes #14453

Checklist

  • I have run make tidy to update any new dependencies
  • I have run make lint to verify my code passes the lint check
    • I have formatted my code using gofumpt
  • I have added tests that prove my fix is effective or that my feature works
  • I have run make changelog and committed the changelog/pending/<file> documenting my change
  • Yes, there are changes in this PR that warrants bumping the Pulumi Cloud API version

@pulumi-bot
Copy link
Contributor

pulumi-bot commented Nov 3, 2023

Changelog

[uncommitted] (2023-11-15)

Features

  • [cli/display] Adds display when policy packs are being loaded
    #14493

@justinvp
Copy link
Member

justinvp commented Nov 4, 2023

I don't think we want to show the policy packs in the table/tree of resources because a policy pack isn't a resource. It doesn't feel quite right having it nested under the stack resource, since it isn't a child resource of the stack, which is what this tree view is meant to show.

Maybe we keep it really simple for now, and simply emit a Loading policy packs... diagnostic for the update? That's likely the simplest and good enough solution.


If there's a good reason not to do that... here are some other ideas/considerations:

Should we show something for this above the table or perhaps below it? And maybe not even break it out into each individual policy pack, but instead have a single line that doesn't display by default, but does become visible if collectively the policy packs are taking longer than N seconds to load. For example "Loading policy packs..." becomes visible if policy packs collectively is taking longer than a few seconds (bonus points for animating the dots and pluralizing "pack" vs "packs" depending on the count).

This is sort of similar in some ways to how we display progress bars for downloading/installing required missing plugins at the start of the pulumi up operation (which, unrelatedly, has a bug in its rendering that we need to fix and make work better when it's displayed as part of pulumi up #14250).

One thing we don't currently do is show any loading indicators when loading other types of plugins like resource plugins (i.e. providers, e.g. aws, azure-native, gcp, kubernetes, etc.). Those are generally relatively fast to load, so there hasn't been a need to do that. It is interesting that we're saying policy packs are different enough to warrant displaying some kind of loading indicator. I suppose this is because it's much more likely for policy packs to have slower startup times, as they're typically written in TS or Python, aren't precompiled, can be using our giant SDKs like azure-native (which themselves have perf issues which lead to slow compile/startup types) and likely aren't optimized for startup time. I suppose with MLCs and convenience APIs for writing resource plugins in other languages, it's possible this could become more of a problem for resource plugins. Should we consider designing this with an eye for how we'd show a similar indicator for any other type of plugin that's taking longer than N seconds to load?

Whatever we do, I'm hoping we won't spend much time on it. If there's something quick we can do here that's just for policy packs, that's preferable.

@Frassle
Copy link
Member

Frassle commented Nov 6, 2023

Maybe we keep it really simple for now, and simply emit a Loading policy packs... diagnostic for the update? That's likely the simplest and good enough solution.

+1

@tgummerer
Copy link
Collaborator Author

Thanks for the feedback! I now went for the simplest way possible just adding some output if policy packs are being loaded. Here's what it looks like:

output

I'm not sure about coloring etc., so happy to change things around still.

@tgummerer tgummerer changed the title wip, add output for running policy packs add output for running policy packs Nov 14, 2023
@tgummerer tgummerer requested a review from a team November 15, 2023 08:04
@tgummerer tgummerer marked this pull request as ready for review November 15, 2023 08:04
Copy link
Member

@Frassle Frassle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@tgummerer tgummerer added this pull request to the merge queue Nov 15, 2023
Merged via the queue into master with commit 529ea58 Nov 15, 2023
44 checks passed
@tgummerer tgummerer deleted the tg/policy-progress-output branch November 15, 2023 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show progress output while policy packs are running
4 participants