-
Notifications
You must be signed in to change notification settings - Fork 50
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
Cargo project manager retry 500 errors. #471
Cargo project manager retry 500 errors. #471
Conversation
@gregorymacharia If you haven't linked your GitHub account with Microsoft, there are some reasons why you might want to consider it: https://docs.opensource.microsoft.com/contributing/microsoft-projects/#contributing-with-linked-vs-unlinked-accounts |
@microsoft-github-policy-service agree company="Microsoft" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Polly is already used as a dependency to handle retry logic in other places. I think it would make sense to leverage that here, I'm noticing a fair bit of repeated logic and similar local variables. Alternately, you could make a helper function that does the retrying with a lambda for the action taken to reduce the duplication.
A location where polly used now for example:
OSSGadget/src/Shared/PackageManagers/TypedManager.cs
Lines 117 to 122 in 7d9fada
public async Task<bool> UriExistsAsync(Uri artifactUri, AsyncRetryPolicy<HttpResponseMessage>? policy = null) | |
{ | |
policy ??= DefaultPolicy; | |
return (await policy.ExecuteAsync(() => CreateHttpClient().GetAsync(artifactUri, HttpCompletionOption.ResponseHeadersRead))).StatusCode == HttpStatusCode.OK; | |
} |
I have gone ahead and switched to polly for handling the retries. Let me know if you have further feedback thanks. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
While investigating a spike of Cargo 500s, Terrapin has seen that periodically Cargo registry returns 500 error responses and OSS Gadget does not retry such requests. This change adds a retry mechanism for operations in Cargo project manager and accompanying unit tests for the changes.