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

review what triggers a retry image download #54

Open
danbjoseph opened this issue Jun 19, 2024 · 1 comment
Open

review what triggers a retry image download #54

danbjoseph opened this issue Jun 19, 2024 · 1 comment

Comments

@danbjoseph
Copy link
Member

leftover from #52

@jayqi:
Looking at how the exceptions are documented and defined:

I think if we're being more judicious, we'd want to retry on: ConnectionError, Timeout, and ChunkedEncodingError for connection errors. Some of the other ones, like Invalid*, indicate a problem with the request content and probably wouldn't be fixed on a retry.

Probably also would be best to be more judicious on which HTTP status codes we retry on. In most cases, 400 errors wouldn't be fixed on a retry, but 500 errors may be.

@dragonejt:
While I agree that there are cases where a retry may not change anything, I also don't see any cases where a retry would actively disrupt our logic flow, so these exceptions should be "safe" to retry even if they still fail. There are also cases where, for example, the Mapillary API times out but actually returns an HTTP 400 Bad Request. I agree that we can punt this to an issue to determine the best exceptions to retry on later.

@jayqi
Copy link
Contributor

jayqi commented Jun 20, 2024

Copying my response on the rationale for why we want to fix this here for visibility:

It's not just about "safe" from runtime errors. Repeating requests that are known to fail (if request is invalid, or 403 or 404 errors), then we (a) will cause the user to waste a bunch of time making repeated failed requests and waiting for the exponential backoff, and (b) incur unnecessary server load on Mapillary, which is bad etiquette.

#52 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants