Allow JSONEncoder to return bytes directly#11989
Allow JSONEncoder to return bytes directly#11989kevinpark1217 wants to merge 6 commits intoaio-libs:masterfrom
JSONEncoder to return bytes directly#11989Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11989 +/- ##
========================================
Coverage 98.76% 98.77%
========================================
Files 128 128
Lines 44881 45006 +125
Branches 2382 2385 +3
========================================
+ Hits 44328 44453 +125
Misses 393 393
Partials 160 160
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
JSONEncoder to return bytes directly
|
@bdraco Do the benchmarks already cover these cases? |
|
IIRC, there was a similar request years ago, rejected. |
|
I think it was #4482 |
webknjaz
left a comment
There was a problem hiding this comment.
No tests? How do we know this keeps working?
With ujson dead, I think we're ready to change this: #10795 (comment) My only question is whether the isinstance() calls here are fine, or we should add a new parameter and avoid the isinstance() checks. I suspect this is one of the performance hot paths for some cases (like websockets on homeassistant?). |
|
Ah, fair. I also had a feeling that I'd prefer having a new API rather than overloading the existing one.. |
|
Thanks for working on this! The use case makes sense. I agree with webknjaz about preferring a new API over
Alternative: Add explicit parallel methods like |
5360cba to
c515669
Compare
c515669 to
cae039d
Compare
This implements the maintainers' suggested approach of creating explicit parallel APIs instead of using isinstance() checks: - Add JSONEncoderBytes type (no default encoder provided) - Add JsonBytesPayload class with required dumps parameter - Add json_bytes_response() function with required dumps parameter - Add send_json_bytes() methods to WebSocketResponse and ClientWebSocketResponse (sends as binary frames, required dumps) - Add json_serialize_bytes parameter to ClientSession (None by default, falls back to json_serialize if not set) - Export new APIs from aiohttp.web This avoids isinstance() overhead in hot paths and provides clear semantics for WebSocket frame types. Users must explicitly provide their bytes-returning encoder. Closes aio-libs#11988
d6e60c5 to
584c004
Compare
There was a problem hiding this comment.
Pull request overview
Adds explicit, bytes-oriented JSON serialization APIs (for serializers like orjson) so callers can avoid str→bytes conversion overhead and keep hot paths free of runtime isinstance() checks.
Changes:
- Introduces
JSONEncoderBytesand new bytes-specific primitives:JsonBytesPayloadandweb.json_bytes_response(). - Adds
send_json_bytes()to server/client WebSocket responses to transmit JSON as binary frames. - Adds
ClientSession(json_serialize_bytes=...)to send request JSON bodies using a bytes-returning encoder, plus corresponding test coverage.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
aiohttp/typedefs.py |
Adds JSONEncoderBytes callable type alias. |
aiohttp/payload.py |
Adds JsonBytesPayload for bytes-returning JSON dumps. |
aiohttp/client.py |
Adds json_serialize_bytes session option and uses JsonBytesPayload when set. |
aiohttp/web_response.py |
Adds json_bytes_response() and exports it. |
aiohttp/web.py |
Re-exports json_bytes_response() from aiohttp.web. |
aiohttp/web_ws.py |
Adds WebSocketResponse.send_json_bytes() for binary JSON frames. |
aiohttp/client_ws.py |
Adds ClientWebSocketResponse.send_json_bytes() for binary JSON frames. |
tests/test_payload.py |
Adds unit tests for JsonBytesPayload. |
tests/test_web_response.py |
Adds tests for web.json_bytes_response(). |
tests/test_web_websocket.py |
Adds error-path tests for WebSocketResponse.send_json_bytes(). |
tests/test_client_ws_functional.py |
Adds functional tests asserting send_json_bytes() uses binary frames. |
tests/test_client_functional.py |
Adds functional test for ClientSession(json_serialize_bytes=...). |
CHANGES/11989.feature.rst |
Adds a towncrier feature fragment documenting the new APIs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Rename JSONEncoderBytes to JSONBytesEncoder for consistency - Use object instead of Any in new API signatures - Make dumps keyword-only in send_json_bytes to match send_json pattern - Fix body check to use `is not None` in json_bytes_response - Alphabetize import ordering
8238796 to
90af396
Compare
bacad25 to
e91411e
Compare
- Add Sphinx doc entries for json_bytes_response(), send_json_bytes() in both WebSocketResponse and ClientWebSocketResponse - Use proper RST cross-reference roles in changelog - Close ws at end of test_send_json_bytes_nonjson - Add orjson to docs spelling wordlist
e91411e to
f56a472
Compare
…tic test values - Revert parameter types from `object` back to `Any` per Dreamsorcerer's guidance that `object` is inappropriate for JSON serializer inputs - Use :class:`str` and :class:`bytes` RST roles in docs - Use static byte values in test_passing_body_only instead of json.dumps()
Dreamsorcerer
left a comment
There was a problem hiding this comment.
I think this looks reasonable to me.
@Dreamsorcerer Thanks! It's ready to be merged at you convenience |
| @@ -0,0 +1,7 @@ | |||
| Added explicit APIs for bytes-returning JSON serializer: | |||
| ``JSONBytesEncoder`` type, ``JsonBytesPayload``, | |||
There was a problem hiding this comment.
would be nice to make these real rst links
Summary
Add explicit APIs for bytes-returning JSON serializers (like
orjson), addressing maintainer feedback to avoidisinstance()checks in hot paths.Changes
JSONEncoderunchanged - still returnsstronlyJSONBytesEncodertype -Callable[[Any], bytes]orjson.dumps)isinstance()checks in hot paths - separate explicit APIs insteadobjectinstead ofAnyfor data parameters in new APIsdumpsis keyword-only insend_json_bytes()methods, matchingsend_json()signatureNew APIs
JSONBytesEncodertype intypedefs.pyJsonBytesPayloadclass - requiresdumpsparameterjson_bytes_response()function - requiresdumpsparametersend_json_bytes()methods onWebSocketResponseandClientWebSocketResponse- sends as binary frames, requiresdumpskeyword argumentClientSession(json_serialize_bytes=...)parameter -Noneby default, falls back tojson_serializeif not setDocumentation
json_bytes_response(),WebSocketResponse.send_json_bytes(), andClientWebSocketResponse.send_json_bytes()Benefits
isinstance()checksCloses #11988