httpcore icon indicating copy to clipboard operation
httpcore copied to clipboard

Add overall timeout support

Open karpetrosyan opened this issue 1 year ago • 8 comments

Todo

  • [x] Update documentation

Hi! It seems we have received many requests for adding total timeout support.

In each place where we are using the operation (read, write, pool, connect) timeout, we should take the minimum value of the operation timeout (timeouts['pool'], for example) and the total timeout value (timeouts['total']). We should also measure the time of each operation and adjust the value of the total timeout accordingly, so we always have the correct total timeout value within the scope of the request.

I'v used the context manager syntax to simplify time measuring, also have added a simple api level test for total timeout, as we don't have any tests for the concrete timeouts

karpetrosyan avatar Jul 13 '24 13:07 karpetrosyan

@tomchristie Any thoughts?

karpetrosyan avatar Aug 06 '24 20:08 karpetrosyan

Thanks @karpetrosyan...

Any thoughts?

  • This is a fantastic piece of functionality to add yes, much needed.
  • Could we get a docs update.
  • Related work that's become apparent to me is that read/write timeouts are the wrong API... a single socket timeout would be a better fit.

lovelydinosaur avatar Aug 07 '24 11:08 lovelydinosaur

I guess that's all from our side. The remaining documentation will be available in the HTTPX docs.

karpetrosyan avatar Aug 09 '24 16:08 karpetrosyan

@karpetrosyan this looks great! Unfortunately this doesn't quit solve my use case as I want to exclude pool timeout from total. How hard would it be make the definition of total customizable? Potential API off the top of my head:

r = httpcore.request(
    "GET",
    "https://www.example.com",
    extensions={"timeout": {
        "read": 1.0, 
        ("connect", "read", "write"): 3.0,
        ("connect", "read", "write", "pool"): 10.0,  # equivalent to "total"
    }}
)

gsakkis avatar Aug 28 '24 12:08 gsakkis

I'm really looking forward to this feature. Could you please ping me after PR is ready?

brosoul avatar Oct 16 '24 12:10 brosoul

@brosoul there is a "subscribe" button at the top right of the page, click it to get notified for updates to this issue.

gsakkis avatar Oct 16 '24 15:10 gsakkis

Thanks so much for your work on this @karpetrosyan.

I've been spending a little time thinking about this and I reckon there's a more fundamental improvement we could be making here. Design discussion over at #982.

lovelydinosaur avatar Nov 26 '24 12:11 lovelydinosaur

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jun 27 '25 03:06 stale[bot]