-
-
Notifications
You must be signed in to change notification settings - Fork 377
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
zulip.Client.call_endpoint: Add retry_on_rate_limit_error. #705
base: main
Are you sure you want to change the base?
Conversation
8094595
to
b870097
Compare
zulip/zulip/__init__.py
Outdated
timeout=timeout, | ||
) | ||
if not retry_on_rate_limit_error or result["result"] == "success": | ||
break |
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.
I'd do this as two conditionals, first the success
one, and then the other one. We like to do this so that code coverage tools can tell you whether we have tests for each independent condition.
zulip/zulip/__init__.py
Outdated
elif "code" in result and result["code"] == "RATE_LIMIT_HIT": | ||
secs = result["retry-after"] | ||
elif "X-RateLimit-Reset" in result: | ||
secs = float(result["X-RateLimit-Reset"]) |
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.
So, I think the retry-after
property is there since the beginning of time for these errors; just not code
. So I think you could do
if 'retry-after' not result:
break
wait_before_retry_secs = result["retry-after"]
logger.warning(...)
time.sleep(...)
b870097
to
0d26501
Compare
@timabbott Thanks for the feedback! 👍 |
0d26501
to
7a293bf
Compare
If the call_endpoint method is called with the "retry_on_rate_limit_error" parameter set to true, wait and retry automatically on rate limit errors. See https://chat.zulip.org/#narrow/stream/378-api-design/topic/ Rate.20limits/near/1217048 for the discussion.
7a293bf
to
ffa4008
Compare
Hi :)
I'd like to suggest a new option for the
call_endpoint
method ofzulip.Client
. It's something I added to my bot. @timabbott suggested to also consider this for the API, see https://chat.zulip.org/#narrow/stream/378-api-design/topic/Rate.20limits/near/1217048 for the discussion.Short description of the patch:
If the
call_endpoint
method is called with theretry_on_rate_limit_error
parameter set to true, wait and retry automatically on rate limit errors.