Support async file objects in content= and files= - #1145
Open
tsimoshka wants to merge 4 commits into
Open
Conversation
Read `anyio`, `trio` and `aiofiles` files with `await read()` rather than iterating a line at a time, and set `Content-Length` when the length is known. Multipart uploads from an async file previously looped indefinitely over un-awaited coroutines; they now stream, or raise `TypeError` on a sync `Client`.
Merging this PR will not alter performance
Performance Changes
Comparing Footnotes
|
Contributor
There was a problem hiding this comment.
Review completed against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Fall back to chunked encoding when an async file reports it can't be rewound, so `Content-Length` can't disagree with the bytes actually sent. Reject text-mode async files passed as `content=`, matching the multipart path. Benchmark async uploads through a descriptor so the length path is covered, and reuse one event loop per benchmark rather than paying loop bootstrap on every sample.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The truthiness guard let an empty `str` read past the binary-mode check, so an empty text-mode file uploaded silently as zero bytes instead of raising.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This is my third attempt at this. This PR resolves #597 and implements chunked upload for
anyio.open_file,trio.open_fileandaiofiles.openwhen used ascontent=parameter forpostandputrequest and implements support for multipart file upload for the same libraries. Previous attempts were encode/httpx#3339 and encode/httpx#3698 againstencode/httpx, both went stale while the library was unmaintained. Most of the code is reworked from encode/httpx#3698, with the review comments left on encode/httpx#3339 applied.Currently
content=with an async file iterates it a line at a time instead of reading in chunks, so a file without newlines gets buffered whole, andfiles=with an async file loops forever on both clients, sinceMultipartStream.__aiter__just delegates to the synciter_chunks()andread()returns a coroutine, which is always truthy.changes
_types.AsyncReadableFileprotocol was added along withis_async_readable_filetype predicate function to detect and perform type narrowing for trio/anyio/aiofiles async files_types.FileContentwas extended to include the_types.AsyncReadableFileprotocol_utils.peek_async_filelike_lengthwas added, async wrappers expose a syncfileno()so the fd can be stat'ed, there's notell()/seek()fallback like the sync version because those would need awaiting_content.AsyncIteratorByteStreamupdated to useawait read()for async files instead of looping over lines_content.encode_contentupdated to attach content length header for async files when it's known upfront_multipart.FileFieldupdated witharender_data, andget_length/render_datamade async-aware, syncClientnow raisesTypeErrorinstead of looping_multipart.MultipartStreamupdated withaiter_chunks, and__aiter__updated to use it, both wrapped incontextlib.aclosingContent-Lengthtaken fromos.fstatwouldn't match the encoded bodytyping_extensionsbumped to >=4.10 forTypeIs(already a dep for python < 3.13), lockfile updatedtest_content.py,test_multipart.pyandtest_utils.py, benchmarks added totest_benchmark.py, docs and changelog updatedperf
content=over 4 MiB with 64-byte lines: 9.18 ms -> 0.33 ms (encoding path, in-memory file, so no disk I/O in the number)io.BytesIO/open()cost 57 ns instead of ~630 ns. Benchmarked against a cleanmainworktree, sync multipart is unchanged within noise.Checklist