Discourse API throttling.

[goetzk]

In the last month or so Discourse implemented API throttling.

I don't have a lot of detail yet but I was told

We recently added rate limiting to api calls which is why you are now running in to some issues. [...] You are allowed 60 requests per minute, so you shouldn’t have to slow things down too much.

Are you open to building the throttling directly in to _request or would you prefer library users handled it individually?
https://github.com/bennylope/pydiscourse/blob/master/pydiscourse/client.py#L1186

At the moment an error along these lines is raised (this is what caused me to ask Discourse).

 pydiscourse.exceptions.DiscourseClientError: We have a daily limit on how many times that action can be taken. Please wait 5 seconds before trying again.

thanks,
kk

[goetzk]

Reference to the relevant announcement.

https://meta.discourse.org/t/global-rate-limits-in-discourse/78612

If you are consuming the API programmatically and receive back a 429 status code you should respect it and slow down.

[bennylope]

If nothing else, it would be good to add a specific Exception class to represent the 429 status. I'd call the the 90% solution.

I'm not strictly opposed to adding throttling, but that sounds like a second step, and a potentially more complicated step with a lot smaller payoff. It'd have to be configurable and/or opt-in, since the limits can be reconfigured per-Discourse deployment.

Are you thinking of adding a rate limiter or an explicit request delay?

[goetzk]

(I replied with this on the 30th, apparently it didn't make it to GitHub. I haven't had time to follow this up any further but may do in the next few weeks).

Good question. Thinking about it, the latter:

I was hoping they sent the number of seconds to delay in the errors
field so it could be easily extracted and waited on ("please wait 10
seconds" -> sleep(10) -> continue). From a quick look errors is simply
returning a string, so the delay length would have to be extracted
before use.

{"errors":["We have a daily limit on how many times that action can be
taken. Please wait 10 seconds before trying
again."],"error_type":"rate_limit"}

thanks,
kk

[goetzk]

Following my request, Discourse have added a separate field to the response which includes the back off period.

I can't test this until they've upgraded our discourse instance, but I'm guessing something like this with a sleep() will be needed.
@bennylope , could you comment on how best to retry the connection after the pause?

diff --git a/pydiscourse/client.py b/pydiscourse/client.py
index 64e2b29..b0831fc 100644
--- a/pydiscourse/client.py
+++ b/pydiscourse/client.py
@@ -1216,7 +1216,14 @@ class DiscourseClient(object):
                     msg = u'{0}: {1}'.format(response.status_code, response.text)
 
             if 400 <= response.status_code < 500:
-                raise DiscourseClientError(msg, response=response)
+                if 429 == response.status_code:
+                    # raise DiscourseRateLimitedError(
+                    rj = response.json()
+                    error_message = rj['errors'][0]     # errors is a list
+                    error_type =  rj['error_type']      # Will be 'rate_limit'
+                    wait_seconds =  rj['extras']['wait_seconds']        # how long to back off for.
+                else:
+                    raise DiscourseClientError(msg, response=response)
 
             raise DiscourseServerError(msg, response=response)
 

[bennylope]

From a quick look errors is simply returning a string, so the delay length would have to be extracted before use.

It'd probably be a good idea to extract - given that it's the format "X seconds" a regex match should suffice.

could you comment on how best to retry the connection after the pause?

In general I'd use a loop (usually a while loop) with a retry count, and keep retrying until that retry count is drawn down to 0, where the retry count is a parameter with a default (e.g. 3).

Consider this a napkin sketch:

while retry_count > 1:
    response = requests.request(...)
    if 400 <= response.status_code < 500:
        if response.status_code == 429:
            logger.warning("429 response message")
            wait_seconds = parse_wait_seconds(response)
            if retry_count > 1:
                time.sleep(wait_seconds)
            retry_count -= 1
            continue  # 
        else:
             # All other 4xx options return or raise
    elif: 
        # All other code options return or raise
        
raise DiscourseRateLimitedError("429!", response=response)

This has a value premise: retry_count is never less than 1 to start.

Every response other than a 429 must either result in a return value or raising an exception from within the while loop, at least as this is written.

429 responses should be logged with a warning, before final success or exception, as this is important information to know for an API user.