-
Notifications
You must be signed in to change notification settings - Fork 15
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
fix: add retries for .taskcluster.yml fetches #30
Conversation
5e68a86
to
db1cd30
Compare
db1cd30
to
abe6f8e
Compare
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
raise e | ||
client = RetryClient( | ||
client_session=aiohttp_session(), | ||
retry_options=ExponentialRetry(attempts=5, statuses={404}), |
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.
It seems odd to me that a 404 would be intermittent; are we sure that's what's happening?
We should probably retry on 5xx though, if we're adding retries...
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.
It seems odd to me that a 404 would be intermittent; are we sure that's what's happening?
As far as I can tell, the only way we can ever end up returning None from tcyml.get
is by hitting this line, which can only happen on a 404.
We should probably retry on 5xx though, if we're adding retries...
Somewhat confusingly, 5xx responses are retried even when statuses are passed:
Important: by default all 5xx responses are retried + statuses you specified as statuses param If you will pass retry_all_server_errors=False than you can manually set what 5xx errors to retry.
I'll add a comment to this effect.
d9e3fd2
to
abfab52
Compare
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.
Intermittent 404s still seem rather wonky, but I don't have a better idea so let's go with this.
We get this intermittently for whatever reason, and it's constantly busting deployments.
abfab52
to
6b078bd
Compare
We get this intermittently for whatever reason, and it's constantly busting deployments.
I tried to debug the 404s a bit but didn't get anywhere; I don't think it's worth digging into more unless we see it with retries.