-
-
Notifications
You must be signed in to change notification settings - Fork 403
zulip.Client.call_endpoint: Add retry_on_rate_limit_error. #705
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
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_endpointmethod 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_endpointmethod is called with theretry_on_rate_limit_errorparameter set to true, wait and retry automatically on rate limit errors.