Skip to content
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

Retry.get_retry_after does't respect time-out #1338

Open
krukas opened this issue Mar 16, 2018 · 3 comments · May be fixed by #3233
Open

Retry.get_retry_after does't respect time-out #1338

krukas opened this issue Mar 16, 2018 · 3 comments · May be fixed by #3233
Labels
💰 Bounty $100 If you complete this issue we'll pay you $100 on OpenCollective! Contributor Friendly ♥

Comments

@krukas
Copy link

krukas commented Mar 16, 2018

Maybe a weird case but if a website gives back an Retry-After that is higher then given timeout it will accept that.

In my case some website gives back a retry of 3600 and so my checkup tasks was waiting for 1 hour to try it one more time.

Maybe ignore large values or at least if Retry-After is bigger then timeout then use timeout or ignore Retry-After

@sethmlarson
Copy link
Member

Could you recreate a testcase using httpbin so we can verify this? :)

@sigmavirus24
Copy link
Contributor

Maybe ignore large values or at least if Retry-After is bigger then timeout

timeout is not a wall-clock timeout. timeout applies to time it takes to connect to the server and time it takes to read the contents.

The logic you're asking for could instead be implemented in a subclass of Retry, if I remember correctly. It's not inherently correct when you consider what timeout does for the library.

@haikuginger
Copy link
Contributor

haikuginger commented Mar 19, 2018

I agree with @sigmavirus24. I would consider a pull request that adds an additional kwarg (something like max_retry_wait_length) that raises an exception (with sufficient information to allow retrying the request after the server-given interval) when the response wants to wait longer than a specified interval. Of course, you can also configure using a Retry object that disables obeying Retry-After headers.

@sethmlarson sethmlarson added this to the v1.24 milestone Jun 9, 2018
@sethmlarson sethmlarson removed this from the v1.24 milestone Jan 23, 2019
@sethmlarson sethmlarson added the 💰 Bounty $100 If you complete this issue we'll pay you $100 on OpenCollective! label Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💰 Bounty $100 If you complete this issue we'll pay you $100 on OpenCollective! Contributor Friendly ♥
Projects
None yet
4 participants