Repository navigation
Build the websocket reader_c extension without the reader_c.py symlink - #13457
Conversation
pyba-chronographer
Bot
added
the
bot:chronographer:provided
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #13457 +/- ##
==========================================
+ Coverage 98.42% 98.99% +0.57%
==========================================
Files 133 132 -1
Lines 49747 49455 -292
Branches 2628 2571 -57
==========================================
- Hits 48961 48960 -1
+ Misses 662 371 -291
Partials 124 124
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
|
pre-commit.ci run |
bdraco
added
backport-3.14
Confidence Score: 5/5The PR appears safe to merge, with no concrete build, runtime, packaging, security, or consumer compatibility defect established. The explicit Cython module name preserves the compiled extension identity, pure-Python fallback remains available when the extension is absent, and the remaining source edit only removes a coverage directive. Reviews (1): Last reviewed commit: "Remove stale no branch pragma from the w..." | Re-trigger Greptile |
There was a problem hiding this comment.
I like this overall.
Though, is this me or does the last in-branch commit come with a bunch of sudden indirect coverage increase? https://app.codecov.io/gh/aio-libs/aiohttp/commit/b145e495a4611ffdbc2fe3318989f5f312a0d7c4/indirect-changes. Is this flaky/unrelated?
| @@ -0,0 +1 @@ | |||
| Removed the ``aiohttp/_websocket/reader_c.py`` symlink from the source tree; the ``aiohttp._websocket.reader_c`` extension is now compiled directly from ``reader_py.py`` using ``cython --module-name``, so distributions no longer include a ``reader_c.py`` file that showed up as an uncovered module in coverage reports -- by :user:`bdraco`. | |||
There was a problem hiding this comment.
| Removed the ``aiohttp/_websocket/reader_c.py`` symlink from the source tree; the ``aiohttp._websocket.reader_c`` extension is now compiled directly from ``reader_py.py`` using ``cython --module-name``, so distributions no longer include a ``reader_c.py`` file that showed up as an uncovered module in coverage reports -- by :user:`bdraco`. | |
| Removed the :file:`aiohttp/_websocket/reader_c.py` symlink from the source tree; the ``aiohttp._websocket.reader_c`` extension is now compiled directly from :file:`reader_py.py` using :command:`cython --module-name`, so distributions no longer include a :file:`reader_c.py` file that showed up as an uncovered module in coverage reports -- by :user:`bdraco`. |
Probably last commit on master failed, so has less coverage. The expected coverage is 98.99%, so it's correct. |
Backport to 3.15: 💚 backport PR created✅ Backport PR branch: Backported as #13469 🤖 @patchback |
3 participants
What do these changes do?
Removes the
aiohttp/_websocket/reader_c.pysymlink; the extension is now compiled directly fromreader_py.pyusingcython --module-name aiohttp._websocket.reader_c. The symlink only existed so Cython would pair the source withreader_c.pxdand emit the right module name, but after #13388 the wheel build materializes it as a regular file in site-packages, which coverage then reports as a new 0% file.Are there changes in behavior for the user?
No runtime changes; dists no longer contain a stray
reader_c.pyfile,reader_conly exists as the compiled extension.Is it a substantial burden for the maintainers to support this?
No, it is one less special case; the Makefile rule is the only thing that changed.
Related issue number
Follow up to #13388
Checklist
CONTRIBUTORS.txtCHANGES/folder<issue_or_pr_num>.<type>.rst(e.g.588.bugfix.rst)Drafted with Claude Code; reviewed by bdraco.