Skip to content

Make wait=true a default option for check and dashboard#1640

Merged
klingerf merged 3 commits intolinkerd:masterfrom
alenkacz:av/wait-as-default
Sep 14, 2018
Merged

Make wait=true a default option for check and dashboard#1640
klingerf merged 3 commits intolinkerd:masterfrom
alenkacz:av/wait-as-default

Conversation

@alenkacz
Copy link
Contributor

resolves #1618

Signed-off-by: Alena Varkockova [email protected]

@klingerf
Copy link
Contributor

@alenkacz Thanks for submitting this! Sorry this wasn't specified in the ticket, but I'd prefer to keep the --wait flag available and change its default value from false to true. I think we should also make the equivalent change for the --wait flag in cli/cmd/dashboard.go.

@wmorgan
Copy link
Member

wmorgan commented Sep 13, 2018

I agree. Sorry @alenkacz I should've been more specific in the issue description!

@olix0r
Copy link
Member

olix0r commented Sep 13, 2018

@klingerf

I'd prefer to keep the --wait flag available and change its default value from false to true

What do you think about --wait taking a duration, with the default being something very high?

I'm imagining that in CI, etc, it would be nice to say --wait 1 minute, for instance -- we'd want the command to fail eventually, just not immediately.

@klingerf
Copy link
Contributor

@olix0r That works for me, and it relates to the change requested in ##1586.

@alenkacz
Copy link
Contributor Author

alenkacz commented Sep 14, 2018

@klingerf ok that actually makes sense :) I will change that. And it's my bad - I did not realize, you can do --wait=false ... I thought it's just --wait (to enable it).

I'll address #1586 in another PR to not mix those two together if it works for everyone.

@alenkacz
Copy link
Contributor Author

@klingerf fixed and added wait=true also for dashboard

@alenkacz alenkacz changed the title Remove wait option and make it a default for check Make wait=true a default option for check and dashboard Sep 14, 2018
Copy link
Contributor

@klingerf klingerf left a comment

Choose a reason for hiding this comment

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

⭐️ Great, thanks for updating @alenkacz. And I agree that it makes sense to tackle #1586 as a separate PR. I'll leave a comment on that issue with some thoughts about how I think we should implement the duration-based flag.

@klingerf klingerf merged commit 169dcf4 into linkerd:master Sep 14, 2018
zachalbert added a commit to zachalbert/linkerd2 that referenced this pull request Sep 15, 2018
* master:
  Move more info from the tap table into the expanded row (linkerd#1641)
  `linkerd check` sends params on version check (linkerd#1642)
  Bikeshed the tap and top icons (linkerd#1637)
  Add link to tap each row in top table (linkerd#1643)
  Bump default check retry time to 5 minutes (linkerd#1645)
  Make wait=true a default option for check and dashboard (linkerd#1640)
  Add version check to Grafana dashboard (linkerd#1638)
  Add data plane check for metrics Prometheus (linkerd#1635)
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.

make --wait the default

4 participants