Skip to content
This repository was archived by the owner on Mar 16, 2024. It is now read-only.

retry app rm on conflict #2292

Merged
merged 1 commit into from
Oct 25, 2023
Merged

retry app rm on conflict #2292

merged 1 commit into from
Oct 25, 2023

Conversation

keyallis
Copy link
Contributor

Checklist

  • The title of this PR would make a good line in Acorn's Release Note's Changelog
  • The title of this PR ends with a link to the main issue being address in parentheses, like: This is a title (#1216). Here's an example
  • All relevant issues are referenced in the PR description. NOTE: don't use GitHub keywords that auto-close issues
  • Commits follow contributing guidance
  • Automated tests added to cover the changes. If tests couldn't be added, an explanation is provided in the Verification and Testing section
  • Changes to user-facing functionality, API, CLI, and upgrade impacts are clearly called out in PR description
  • PR has at least two approvals before merging (or a reasonable exception, like it's just a docs change)

retry app rm on conflict
39867c5 
Signed-off-by: Oscar Ward <oscar@acorn.io>
cjellick
cjellick reviewed Oct 25, 2023
View reviewed changes
pkg/cli/rm_helper.go
@@ -123,7 +125,11 @@ func deleteOthers(ctx context.Context, c client.Client, arg string) (bool, error
}

func removeAcorn(ctx context.Context, c client.Client, arg string, ignoreCleanup, wait bool) error {
app, err := c.AppDelete(ctx, arg)
var app *v1.App
err := retry.RetryOnConflict(retry.DefaultRetry, func() (err error) {
Copy link
Member

Choose a reason for hiding this comment

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

does this retry just once or a bunch of times?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's configured to retry with the following configuration:

var DefaultRetry wait.Backoff = wait.Backoff{
     Steps:    5,
     Duration: 10 * time.Millisecond,
     Factor:   1.0,
     Jitter:   0.1,
 }

@keyallis keyallis requested a review from cjellick October 25, 2023 19:03
@keyallis keyallis merged commit 86e5ba2 into acorn-io:main Oct 25, 2023
@keyallis keyallis deleted the app-rm-retry branch October 25, 2023 19:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@thedadams thedadams Awaiting requested review from thedadams

@cjellick cjellick Awaiting requested review from cjellick

3 more reviewers

@njhale njhale njhale approved these changes

@g-linville g-linville g-linville approved these changes

@tylerslaton tylerslaton tylerslaton approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@keyallis @cjellick @njhale @g-linville @tylerslaton