test(python-sdk): add regression tests for template upload Content-Length#1292
test(python-sdk): add regression tests for template upload Content-Length#1292mishushakov wants to merge 1 commit intomainfrom
Conversation
…ngth Guards against future changes swapping the buffered bytes for a generator/stream, which would trigger Transfer-Encoding: chunked and break S3 presigned PUT uploads with 501 NotImplemented. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PR SummaryLow Risk Overview The tests also guard against future changes that would switch the request body to a stream/generator and trigger Reviewed by Cursor Bugbot for commit bc3f865. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
Package ArtifactsBuilt from 3d860d5. Download artifacts from this workflow run. JS SDK ( npm install ./e2b-2.19.1-fix-python-sdk-upload-content-length-tests.0.tgzCLI ( npm install ./e2b-cli-2.9.1-fix-python-sdk-upload-content-length-tests.0.tgzPython SDK ( pip install ./e2b-2.20.0+fix.python.sdk.upload.content.length.tests-py3-none-any.whl |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc3f865330
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| class Handler(BaseHTTPRequestHandler): | ||
| def do_PUT(self): | ||
| state["headers"] = dict(self.headers) |
There was a problem hiding this comment.
Converting self.headers to a plain dict here drops the case-insensitive behavior of HTTPMessage, so later checks like .get("Content-Length") / .get("Transfer-Encoding") can fail if the client sends lowercase header names (which is valid HTTP). In that case this regression test would report a false failure even though Content-Length is present and correct, making CI flaky across client/library versions.
Useful? React with 👍 / 👎.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — test-only regression guard, no SDK behavior change.
Extended reasoning...
Overview
Adds two new test files (sync + async) under packages/python-sdk/tests/ that spin up a local http.server.HTTPServer in a daemon thread and assert that upload_file sends a Content-Length header matching the received body and does not use Transfer-Encoding: chunked. No production SDK code is touched.
Security risks
None. The tests bind to 127.0.0.1 on an ephemeral port and only run within the test suite. No credentials, network egress, or new dependencies.
Level of scrutiny
Low. Test-only additions that guard against a specific S3 presigned PUT regression (501 NotImplemented on chunked encoding, mirroring the JS SDK fix in #1285/#1243). The tests are self-contained, mechanical, and well-commented.
Other factors
The bug hunting system found no issues. Minor potential for test flakiness from thread shutdown exists, but the server.shutdown() + thread.join(timeout=5) pattern in a finally block is standard. No changeset is needed since this doesn't affect published behavior.
1 participant
Summary
upload_fileintemplate_sync/template_asyncthat spin up a local HTTP server and assert the PUT request carries a validContent-Lengthand does not useTransfer-Encoding: chunked.tar_buffer.getvalue()) for a generator/stream, which would trigger chunked encoding and break S3 presigned PUT uploads with 501 NotImplemented (the same class of bug that hit the JS SDK in fix(js-sdk): buffer template upload to avoid S3 chunked PUT 501 #1285 / [JS SDK] uploadFile uses chunked transfer encoding, causing 501 NotImplemented on S3 presigned PUT URLs #1243).No Python SDK code changes — the current implementation already passes bytes to
httpx.put(..., content=...), so this is purely a regression guard.Test plan
poetry run pytest tests/sync/template_sync/test_upload_file.py tests/async/template_async/test_upload_file.py -v— both pass locallypoetry run ruff check/ruff format --checkclean🤖 Generated with Claude Code