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

Reuse conflict retry loop from client-go/util/retry #1102

Closed
rhuss opened this issue Nov 9, 2020 · 7 comments · Fixed by #1441
Closed

Reuse conflict retry loop from client-go/util/retry #1102

rhuss opened this issue Nov 9, 2020 · 7 comments · Fixed by #1441
Assignees
Labels
good first issue Denotes an issue ready for a new contributor. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. triage/accepted Issues which should be fixed (post-triage)

Comments

@rhuss
Copy link
Contributor

rhuss commented Nov 9, 2020

Instead of our homegrown algorithm for retrying updates in case of a conflict, we should reuse https://github.com/kubernetes/client-go/blob/1eb2027cd51eefaca0f851d5a2c90f8dd9b2a71d/util/retry/util.go#L68-L75 from the kubernetes client lib.

The goal of this refactoring task is to identify the spots where we are doing retries and switch over to this utility method, much like the kn service import pr does.

@rhuss rhuss changed the title Reuse Reuse conflict retry loop from client-go/util/retry Nov 11, 2020
@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 10, 2021
@rhuss
Copy link
Contributor Author

rhuss commented Feb 15, 2021

/remove-lifecycle stale

I think this is still a good idea to move this feature to a shared place

@knative-prow-robot knative-prow-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Feb 15, 2021
@github-actions
Copy link

This issue is stale because it has been open for 90 days with no
activity. It will automatically close after 30 more days of
inactivity. Reopen the issue with /reopen. Mark the issue as
fresh by adding the comment /remove-lifecycle stale.

@github-actions github-actions bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 17, 2021
@rhuss
Copy link
Contributor Author

rhuss commented Jun 1, 2021

/remove-lifecycle stale
/good-first-issue

Still a valid improvement to reduce our code footprint. Also, I think this would be a good first issue.

@knative-prow-robot
Copy link
Contributor

@rhuss:
This request has been marked as suitable for new contributors.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-good-first-issue command.

In response to this:

/remove-lifecycle stale
/good-first-issue

Still a valid improvement to reduce our code footprint. Also, I think this would be a good first issue.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@knative-prow-robot knative-prow-robot added good first issue help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 1, 2021
@rhuss rhuss added the triage/accepted Issues which should be fixed (post-triage) label Jul 9, 2021
@rhuss rhuss added good first issue Denotes an issue ready for a new contributor. and removed good first issue labels Jul 9, 2021
@dsimansk
Copy link
Contributor

On top of the original proposal. There might be a good enhancement to use the conflict retry loop in other update operations, e.g. sources, triggers, etc. to avoid cases like we see in E2E tests.

https://prow.knative.dev/view/gs/knative-prow/pr-logs/pull/knative_client/1419/pull-knative-client-integration-tests/1425048794432540672#1:build-log.txt%3A839

@vyasgun
Copy link
Contributor

vyasgun commented Aug 27, 2021

/assign

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. triage/accepted Issues which should be fixed (post-triage)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants