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

Remove redundant retry from job handler #2695

Closed
wants to merge 1 commit into from

Conversation

georgethebeatle
Copy link
Member

Is there a related GitHub Issue?

No

What is this change about?

Remove redundant retry logic from the job handler

The purpose of jobs is to report async operation statuses to the cli or
whatever other clients may be polling for these statuses. Therefore
there is no need to retry anything in this handler as the clients
themselves will retry.

Does this PR introduce a breaking change?

No

Acceptance Steps

Green tests

Tag your pair, your PM, and/or team

@gcapizzi

The purpose of jobs is to report async operation statuses to the cli or
whatever other clients may be polling for these statuses. Therefore
there is no need to retry anything in this handler as the clients
themselves will retry.

Co-authored-by: Georgi Sabev <[email protected]>
@davewalter
Copy link
Member

When we were first implementing the org deletion job endpoint, we found that the first request to the /v3/jobs endpoint made by the CLI immediately after deleting a resource occasionally returned a 404 error due to the deletion timestamp appearing to still be unset. The CLI considered this to be an exception and stopped polling. It didn't happen very often, but enough to make us concerned. Is there a better way to avoid this without this retry logic?

@georgethebeatle
Copy link
Member Author

Thanks for your reply @davewalter, I was told about this reasoning at Friday end of day. I think it is fair and given that no one will really intentionally try to get the deletion job status for a non existent job I think we can just leave it like it is.

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.

3 participants