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

feat: wait flag for apps create-deployment command #969

Merged
merged 4 commits into from Mar 19, 2021
Merged

feat: wait flag for apps create-deployment command #969

merged 4 commits into from Mar 19, 2021

Conversation

Shanduur
Copy link
Contributor

@Shanduur Shanduur commented Mar 18, 2021

In this pull request issue #964 is addressed:

  • --wait flag was added to the doctl apps create-deployment
  • defaults to false
  • waits for deployment to enter ACTIVE phase
  • on phases ERROR, CANCELED and UNKNOWN returns error

@Shanduur Shanduur changed the title feat(#964): wait flag for apps create-deployment command feat: wait flag for apps create-deployment command Mar 19, 2021
@Shanduur Shanduur marked this pull request as ready for review Mar 19, 2021
@ChiefMateStarbuck ChiefMateStarbuck requested a review from Mar 19, 2021
Copy link
Member

@andrewsomething andrewsomething left a comment

This is great stuff @Shanduur! Thanks for the contribution!

To answer your question from #964, it would be good to include this in the integration testing. Are you up for it?

@Shanduur
Copy link
Contributor Author

@Shanduur Shanduur commented Mar 19, 2021

Maybe in future PR, but I would have to check how are you doing the integration testing - I tried passing it straight away, but (as I suppose) this caused output mismatch. Additionally the response time is not deterministic, so this should probably be also took into account. As doctl kubernetes cluster create does not include --wait flag in integration testing, I skipped this part for now.

Copy link
Member

@andrewsomething andrewsomething left a comment

👍 I pushed some additional commits adding basic tests that exercise the feature a bit. Take a look if you want to see the ways we are currently testing.

Thanks again for this great contribution!

@andrewsomething andrewsomething merged commit 5d5abeb into digitalocean:main Mar 19, 2021
5 checks passed
@MattIPv4
Copy link
Member

@MattIPv4 MattIPv4 commented Mar 24, 2021

Hey, @Shanduur - Thanks a ton for this sweet PR! 😄

Would you please shoot me an email when you get a chance?
mcowley at digitalocean dot com 🎉

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.

None yet

3 participants