Skip to content

Add .elapsed onto Response objects tracking how long a request took. - #351

Merged
florimondmanca merged 4 commits into
encode:masterfrom
rowillia:ISSUE_338_add_elapsed_to_response
Sep 18, 2019
Merged

florimondmanca merged 4 commits into
encode:masterfrom
rowillia:ISSUE_338_add_elapsed_to_response

Conversation

@rowillia

Copy link
Copy Markdown
Contributor

This should fix #338

Example:

import asyncio

import httpx

async def await_url(c):
    r = await c.get('http://httpbin.org/get')
    await r.read()
    print(r.elapsed)

async def httpx_test():
    client = httpx.AsyncClient()
    await asyncio.gather(*[await_url(client) for _ in range(25)])

asyncio.run(httpx_test())

This should fix encode#338

Example:

```python
import asyncio

import httpx

async def await_url(c):
    r = await c.get('http://httpbin.org/get')
    await r.read()
    print(r.elapsed)

async def httpx_test():
    client = httpx.AsyncClient()
    await asyncio.gather(*[await_url(client) for _ in range(25)])

asyncio.run(httpx_test())
```
@florimondmanca

florimondmanca commented Sep 17, 2019 •

Copy link
Copy Markdown
Contributor

Thanks for tackling this @rowillia! I’ll give this a review later. :)

(Edited) One question I had — did you investigate if it was possible to call the elapsed time tracker at a higher level, eg in BaseClient._get_response()? The response object in there is obtained when the headers have been received, which seems to be what we want to be timing for. Of course we'd only get an approximation because elapsed would also include our own processing time, but that's probably negligible anyway? Then we can just set response.elapsed instead of passing it as an argument. The main motivation is to centralize the timing logic in one place, making it less bug-prone and more easy to extend/maintain.

@florimondmanca
florimondmanca self-requested a review September 17, 2019 07:20
@rowillia

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @florimondmanca ! I went ahead and moved that logic into _get_response.

@rowillia
rowillia force-pushed the ISSUE_338_add_elapsed_to_response branch from 12a436b to 979200d Compare September 17, 2019 17:06
@sethmlarson

sethmlarson commented Sep 17, 2019 •

Copy link
Copy Markdown
Contributor

Btw when we document this let's put a warning to use timedelta.total_seconds() instead of .seconds. This is a common issue that new users of the datetime module hit.

@florimondmanca florimondmanca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great stuff! I left a couple of polishing suggestions. :)

Comment thread tests/test_utils.py Resolved
Comment thread tests/models/test_responses.py Resolved
Comment thread tests/test_api.py Resolved
Comment thread docs/api.md Resolved

@florimondmanca florimondmanca left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome 🎉 Just a tiny change you can commit via the suggestion, and then we're good to merge I think!

Comment thread tests/test_api.py Resolved
Co-Authored-By: Florimond Manca <florimond.manca@gmail.com>
@rowillia

Copy link
Copy Markdown
Contributor Author

Thanks for the feedback @florimondmanca ! LMK if there's anything else to update.

@florimondmanca
florimondmanca merged commit 05ac117 into encode:master Sep 18, 2019
@florimondmanca

Copy link
Copy Markdown
Contributor

I think we're good — merged! Thank you @rowillia. 💯

Kludex referenced this pull request in pydantic/httpx2 May 11, 2026
Bumps [ghp-import](https://github.com/c-w/ghp-import) from 1.1.0 to 2.0.1.
- [Release notes](https://github.com/c-w/ghp-import/releases)
- [Commits](c-w/ghp-import@1.1.0...2.0.1)

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Tom Christie <tom@tomchristie.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Missing requests' elapsed property on Response object.

3 participants