bug: forward files in async _build_request so async post uploads keep the multipart body#437
Open
anxkhn wants to merge 1 commit into
Open
bug: forward files in async _build_request so async post uploads keep the multipart body#437anxkhn wants to merge 1 commit into
anxkhn wants to merge 1 commit into
Conversation
The async AsyncAPIClient._build_request omitted the files argument when calling httpx build_request, so multipart file uploads made through the async post proxy (AsyncPortkey.post(url=..., files=...)) silently dropped the file body. The sync APIClient._build_request already passes files=options.files; mirror it on the async path so async and sync behave identically. Add offline regression tests that assert the outgoing request carries a multipart/form-data body for both the sync and async post file paths. Signed-off-by: Anas Khan <83116240+anxkhn@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a parity bug between sync and async clients where async post(..., files=...) dropped multipart file bodies by not forwarding files into the underlying httpx request builder, and adds regression tests to prevent reintroduction.
Changes:
- Forward
options.filesinAsyncAPIClient._build_request(...)to ensure multipart uploads are preserved. - Add sync + async regression tests using
httpx.MockTransportto assert outgoing requests are multipart and include the uploaded filename/bytes.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
portkey_ai/api_resources/base_client.py |
Adds files=options.files to async request construction to match sync behavior. |
tests/test_post_files.py |
Adds offline regression coverage for sync/async multipart file uploads via the POST proxy. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+22
to
+38
| def test_sync_post_sends_multipart_files() -> None: | ||
| captured: Dict[str, object] = {} | ||
| http_client = httpx.Client( | ||
| transport=_capture_transport(captured), base_url=BASE_URL | ||
| ) | ||
| client = Portkey(api_key="dummy", base_url=BASE_URL, http_client=http_client) | ||
|
|
||
| client.post( | ||
| url="/files", | ||
| purpose="assistants", | ||
| files={"file": ("hello.txt", b"hello world", "text/plain")}, | ||
| ) | ||
|
|
||
| assert "multipart/form-data" in captured["content_type"] | ||
| body = captured["content"] | ||
| assert b"hello.txt" in body | ||
| assert b"hello world" in body |
Comment on lines
+41
to
+58
| @pytest.mark.asyncio | ||
| async def test_async_post_sends_multipart_files() -> None: | ||
| captured: Dict[str, object] = {} | ||
| http_client = httpx.AsyncClient( | ||
| transport=_capture_transport(captured), base_url=BASE_URL | ||
| ) | ||
| client = AsyncPortkey(api_key="dummy", base_url=BASE_URL, http_client=http_client) | ||
|
|
||
| await client.post( | ||
| url="/files", | ||
| purpose="assistants", | ||
| files={"file": ("hello.txt", b"hello world", "text/plain")}, | ||
| ) | ||
|
|
||
| assert "multipart/form-data" in captured["content_type"] | ||
| body = captured["content"] | ||
| assert b"hello.txt" in body | ||
| assert b"hello world" in body |
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.
Title: Forward
filesin the async_build_requestso async POST file uploads send the multipart bodyDescription:
AsyncAPIClient._build_requestcalledhttpxbuild_requestwithoutfiles=options.files, while the syncAPIClient._build_requestpasses it. As aresult, multipart uploads made through the async POST proxy
(
AsyncPortkey.post(url=..., files=...)) silently dropped the file body and sent aplain JSON request instead. The identical sync call already worked.
files=options.filesto theself._client.build_request(...)call in theasync
_build_request, in the same argument position as the sync path, so the twocode paths are byte-for-byte identical.
tests/test_post_files.py) that usehttpx.MockTransportto capture the outgoing request and assert it carries amultipart/form-datacontent type with the file name and bytes in the body, forboth the sync and async POST file paths.
Motivation:
files=support was added to the POST method for the sync client in commit27312b5("feat: accepting files in post method") but was never mirrored on the async client,
so async and sync diverged:
AsyncPortkey.post(url=..., files=...)looked like ituploaded a file but sent no file body. The sync and async clients are meant to behave
identically, and file uploads (for example to a
/filesendpoint) are a normal use ofthe POST proxy. This restores parity with a one-line change.
Related Issues:
None (self-reported bug).
How to verify (reviewer copy/paste)
Regression proof: remove only the added
files=options.files,line from the async_build_requestand re-run the tests. The async test fails withassert 'multipart/form-data' in 'application/json'(the request falls back to a JSONbody, dropping the file); the sync test still passes. Restore the line and both pass.