-
Notifications
You must be signed in to change notification settings - Fork 1
Use exponential backoff for verifying uploads #25
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
Conversation
0ef7771 to
cf42ab5
Compare
| end | ||
| end | ||
|
|
||
| sleep(2) unless uploads.empty? |
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.
https://developers.phrase.com/api/#overview--rate-limiting
In theory you can make 1 request every 3.333 seconds and not hit the rate limit. So I think if you actually just bumped this line to 4 it would maintain okay.
This says "per user" so I'm not sure if they really mean per client.
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.
you actually just bumped this line to 4 it would maintain okay
This is doing up to a request per language each time around the loop, so it's a bit multiplied out.
This says "per user" so I'm not sure if they really mean per client.
It's a complete assumption, but I was assuming they meant "per API key".
| # If we bail mid-sync, it can be expensive to recover from a partial | ||
| # merge. Instead, aggressively try to retry. | ||
| if retries >= 0 | ||
| retries -= 1 | ||
| STDERR.puts('Rate limited, retrying in 5...') | ||
| sleep(5) | ||
| retry | ||
| end |
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.
Technically they return a X-Rate-Limit-Reset which tells you when the rate limit should be okay again 🤷 It's not clear to me if 5 seconds is sufficient or not. If the reset goes back to 100% after 5 minutes though I wouldn't see much harm in just sleeping for 5 minutes.
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.
5 seconds may well be insufficient, but even if each failed request is itself counted towards the rate-limit, it's still slow enough that the pool will eventually drain and we'll be able to make the request. So it might be somewhat lazy behaviour, but I think it's safe.
c68b30e to
00e76fa
Compare
If Phraseapp is slow to process uploads, we can hit the rate limit while waiting to verify them.
352170c to
25bf28a
Compare
If Phraseapp is slow to process uploads, we can hit the rate limit while waiting to verify them.