More gracefully handle exceeding the API limit. #985
Conversation
|
I have been meaning to implment Polly: http://www.thepollyproject.org/ Instead of maintaining our own throttling mechanism. Would recquire a bit of a rework but I think it would be worth it in the longer term. I had a stab at it but I got distracted xD |
|
The responses and request restrictions around the trade API have changed significantly since we last updated it in Procurement, details here. If we're going to refactor Procurement's throttling code, then I'd like to take this into account since we can now actually know up front if we should throttle back or not. |
|
If I'm not mistaken, those rate-limiting headers are for the trade API, which appears to be separate from the stash/inventory requests Procurement makes. On the other hand, Procurement handles the current rate limits well; that is, our current values are still the most aggressive we can be without getting throttled. (I have been using this PR locally, and when I have another tool make API requests at the same time Procurement is, Procurement is throttled very quickly and reliably, which means Procurement fully utilizes the allowed request limits, but is never throttled when running by itself.) |
|
@Stickymaddness , could we merge this PR? I know there are additional, more complex, features we could add to the throttler, but this is a simple code change that greatly improves Procurement's behavior in cases where it does go over the API limit (which, as I said, does not happen during the normal operation of Procurement in isolation). |
If another tool that uses the GGG API is used alongside Procurement, it is very easy to exceed the API limit. This change more gracefully handles that case, but is very cautious about it. If an overflow is detected, completely fill up the window of active tasks, and wait for at least as long as the window size.
thailyn
force-pushed
the
handle-exceeding-api-limit
branch
from
1bf04dc
to
f10e22a
Compare
| HttpWebResponse response = null; | ||
| // TODO: Don't retry an infinite number of times. | ||
| bool retry = true; | ||
| while (retry) |
There was a problem hiding this comment.
Can we at least make this a controlled number of attempts, rather than an infinite loop.
|
I have a lot of reservations about this change, given that it's essentially adding support for running it alongside other applications, as common as that may be these days. Precomputing the throttling as we are currently isn't the best way to go about it either. The headers I linked in a previous comment come back for all requests, not just the ones mentioned in the documentation, and is the correct way of handling the throttling. But, this is unfortunately not something that I have time to implement myself. Can we at least find a smarter way to handle the retries, and not loop indefinitely. |
I've started using other tools that use SESSIONID, alongside Procurement. They don't play well together. This is an initial stab at making Procurement work better when it unexpectedly encounters 429 errors from the GGG API.