fix: unbreak Windows wheel build and bump python-datamodel floor#1517
Merged
Conversation
The CIBW_TEST_COMMAND added recently made cibuildwheel pip-install the built wheel (with all deps) in a test venv on Windows, which pulled in uvloop transitively via python-datamodel and failed: uvloop has no Windows support. Verify the compiled Cython extension by inspecting the wheel archive directly instead of installing it, so dependencies are never resolved just to run the check. Bump the python-datamodel floor to >=0.10.21, the release that marks uvloop as non-Windows-only. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Reviewer's GuideAdjusts the release workflow to verify that built wheels contain the compiled Cython extension by inspecting wheel archives directly instead of installing them via cibuildwheel, and raises the python-datamodel minimum version to one that no longer pulls in uvloop on Windows. Flow diagram for wheel build and Cython extension verification in release workflowflowchart TD
gha[GitHubActions release workflow] --> cbw[cibuildwheel build wheels]
cbw --> dist_dir[dist directory with .whl files]
dist_dir --> verify[Verify compiled Cython extension in wheels]
verify --> loop{each wheel in dist/*.whl}
loop --> inspect[zipfile.ZipFile.namelist]
inspect --> check{asyncdb/utils/types.*.so or .pyd present?}
check -- yes --> ok[print OK and continue]
check -- no --> fail[assertion failure, build fails]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In the wheel verification step, consider using a context manager for
zipfile.ZipFile(e.g.,with zipfile.ZipFile(whl) as zf:) to ensure files are properly closed and avoid potential resource warnings in longer runs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the wheel verification step, consider using a context manager for `zipfile.ZipFile` (e.g., `with zipfile.ZipFile(whl) as zf:`) to ensure files are properly closed and avoid potential resource warnings in longer runs.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The CIBW_TEST_COMMAND added recently made cibuildwheel pip-install the built wheel (with all deps) in a test venv on Windows, which pulled in uvloop transitively via python-datamodel and failed: uvloop has no Windows support.
Verify the compiled Cython extension by inspecting the wheel archive directly instead of installing it, so dependencies are never resolved just to run the check. Bump the python-datamodel floor to >=0.10.21, the release that marks uvloop as non-Windows-only.
Summary by Sourcery
Ensure built wheels contain the compiled Cython extension without installing them and update a core dependency version floor.
Bug Fixes:
Enhancements:
Build: