Skip to content
This repository has been archived by the owner on Jan 8, 2024. It is now read-only.

cli/status: Add a '-refresh' flag to regenerate project applications status on request #2081

Merged
merged 9 commits into from
Aug 25, 2021

Conversation

briancain
Copy link
Member

@briancain briancain commented Aug 20, 2021

This pull request introduces a new flag, -refresh, which will refresh all of the requested status reports for the current view from running waypoint status. This means if you refresh a refresh for a project view, all of that projects applications will be refreshed. If you request a single app, only that app will be refreshed, etc.

This commit introduces a new flag, `-refresh`, which will refresh all of
the requested status reports for the current view from running `waypoint
status`. This means if you refresh a refresh for a project view, all of
that projects applications will be refreshed. If you request a single
app, only that app will be refreshed, etc.
This commit adds a new package for CLI callers to watch an already
queued jobs output and stream it directly to the configured UI terminal.
@briancain briancain requested a review from a team August 20, 2021 15:24
@briancain briancain added this to the 0.5.x milestone Aug 20, 2021
internal/cli/status.go Outdated Show resolved Hide resolved
Copy link
Contributor

@mitchellh mitchellh left a comment

Choose a reason for hiding this comment

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

I think my big comments here is that the CLI subcommand is painfully aware and logic-divergent on whether this is "local" or "remote". All our other tasks also have this distinction, but this is handled at the CLI layer (does it go to a local runner, or remote?).

My opinion is that the CLI implementation should probably just schedule a status report refresh job no matter what and let the local vs remote runner handle that properly. The machinery all gets wired through automatically. This is how every other operation works afaik.

internal/cli/status.go Outdated Show resolved Hide resolved
Copy link
Contributor

@krantzinator krantzinator left a comment

Choose a reason for hiding this comment

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

There's an error case that needs handled, I believe. See comments.

internal/cli/status.go Outdated Show resolved Hide resolved
internal/cli/status.go Show resolved Hide resolved
internal/cli/status.go Outdated Show resolved Hide resolved
internal/cli/status.go Outdated Show resolved Hide resolved
internal/cli/status.go Outdated Show resolved Hide resolved
Co-authored-by: Rae Krantz <8461333+krantzinator@users.noreply.github.com>
No longer required
This commit introduces a new CLI option, WithOptionalApp, which allows
CLIs like `status` to optionally be initialized with a config and a
project client without a hard requirement on an app existing. It now
uses DoApp directly to run a status report job. If the project is
remote, and the CLI is not inside the project dir, it sets the ref app
for the project client to use when queueing the status report job.
@@ -101,6 +102,15 @@ func (c *StatusCommand) Run(args []string) int {
c.ui.Output(wpAppFlagAndTargetIncludedMsg, terminal.WithWarningStyle())
}

// Optionally refresh status
if c.flagRefreshAppStatus {
if err := c.RefreshApplicationStatus(projectTarget, appTarget); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Potential optimization: if they specify a -app flag, we don't need to refresh status on every app in the project.

Copy link
Member Author

Choose a reason for hiding this comment

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

That should happen already!

Copy link
Contributor

@krantzinator krantzinator left a comment

Choose a reason for hiding this comment

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

Gave it a whirl again, and my main points of confusion are more related to how Waypoint manages apps and projects. Which is something I need to fix 😅
-refresh works well!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants