-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Separate aiohttp.http_websocket
into multiple files
#9542
Conversation
There are not functional changes here. The goal is to be able to able to make it easier to build a Cython implementation for the WebsocketReader
There are not functional changes here. The goal is to be able to able to make it easier to build a Cython implementation for the WebsocketReader
There are not functional changes here. The goal is to be able to able to make it easier to build a Cython implementation for the WebsocketReader
There are not functional changes here. The goal is to be able to able to make it easier to build a Cython implementation for the WebsocketReader
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found. Additional details and impacted files@@ Coverage Diff @@
## master #9542 +/- ##
==========================================
- Coverage 98.59% 98.57% -0.03%
==========================================
Files 108 112 +4
Lines 35207 35222 +15
Branches 4184 4184
==========================================
+ Hits 34714 34720 +6
- Misses 330 339 +9
Partials 163 163
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
CodSpeed Performance ReportMerging #9542 will not alter performanceComparing Summary
|
I'm torn on a backport 3.10 as well as otherwise it's going to make future backports harder but I think 3.10 is so short lived that the risk of breakage from users unexpectedly importing internals from http_websocket probably outweighs the small overhead of increased backport conflicts so I didn't tag it for 3.10 as well |
Even though it's an internal change, only, I think it warrants a change log message |
aiohttp.http_websocket
into multiple files
aiohttp.http_websocket
into multiple filesaiohttp.http_websocket
into multiple files
This reverts commit b74b874.
Backport to 3.11: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply dd9a1fd on top of patchback/backports/3.11/dd9a1fdf568de33bc444779ca5a273713223666c/pr-9542 Backporting merged PR #9542 into master
🤖 @patchback |
(cherry picked from commit dd9a1fd)
What do these changes do?
There are no functional changes here. The goal is to be able to able to make it easier to build a Cython implementation for the
WebsocketReader
The idea is to be able to use a PXD file to augment the python code with types so cython can build an optimized version +~88% of it as an extension without having to maintain a separate PYX file #9543
I considered an alternate approach of adding a PYX version which would be a tiny bit faster, but the downside of having to maintain the same code twice and requiring significant Cython knowledge to work on the WebSocket reader lead me to a PXD only approach since only Cython types have to be maintained which has a much lower learning curve.
Note that there are a few lines missing coverage. I did not add any coverage here as the goal was to ensure the only change here was code relocation.
Are there changes in behavior for the user?
no, unless someone was importing internals which is why I added a changelog entry for an internal change.