-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Add decode_text parameter to WebSocket for receiving TEXT as bytes #11764
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #11764 +/- ##
========================================
Coverage 98.74% 98.75%
========================================
Files 127 127
Lines 43879 44171 +292
Branches 2337 2342 +5
========================================
+ Hits 43327 43619 +292
Misses 392 392
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. |
|
now we have a problem with json.loads not decoding bytes |
|
Ok hundreds of lines of typing is not worth it for this change |
CodSpeed Performance ReportMerging #11764 will not alter performanceComparing Summary
|
Co-authored-by: Sam Bull <[email protected]>
Co-authored-by: Sam Bull <[email protected]>
for more information, see https://pre-commit.ci
…to no_decode_websocket_option
|
Should probably skip the 3.13 backport, as this is a significant new feature and will also likely have plenty of conflicts with new syntax etc. |
|
Thanks |
|
The backport is proving tricky. I'll merge this one I have more of it worked out locally |
Backport to 3.14: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 8b919d3 on top of patchback/backports/3.14/8b919d30a2339d3b9dbc53e8958f745b0f9703ea/pr-11764 Backporting merged PR #11764 into master
🤖 @patchback |
…11764) Co-authored-by: Sam Bull <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit 8b919d3)
…11764) Co-authored-by: Sam Bull <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit 8b919d3)
…11764) Co-authored-by: Sam Bull <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit 8b919d3)
…11764) Co-authored-by: Sam Bull <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit 8b919d3)
…11764) Co-authored-by: Sam Bull <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit 8b919d3)
…11764) Co-authored-by: Sam Bull <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> (cherry picked from commit 8b919d3)
…11764) (#11858) Co-authored-by: Sam Bull <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
What do these changes do?
Adds a
decode_textparameter tows_connect()andWebSocketResponse()that allows receiving WebSocket TEXT messages as raw bytes instead of decoded strings.When
decode_text=False:bytesinstead ofstrorjsonthat accept bytesThis addresses the use case where applications want to avoid the wasteful decode step when using JSON parsers that can process bytes directly.
Are there changes in behavior for the user?
No breaking changes. The default
decode_text=Truepreserves existing behavior.New behavior when
decode_text=False:receive()returnsWSMessageTextBytes(withbytesdata) for TEXT messagesreceive_str()returnsbytesinstead ofstrreceive_json()passes bytes to theloadsfunctionIs it a substantial burden for the maintainers to support this?
Low burden. The implementation:
WebSocketReaderWSMessageTextBytestypetyping_extensionsdependency for Python < 3.13Why
typing_extensions?We use
TypeVarwithdefault=parameter (PEP 696, Python 3.13+) to makeWebSocketResponseandClientWebSocketResponsegeneric without breaking existing code. Without the default, all existing code would need to change from:to:
With
default=Literal[True], the existing code continues to work unchanged while allowing explicit typing fordecode_text=False:Related issue number
Fixes #11763
Checklist
CONTRIBUTORS.txtCHANGES/folder