-
Notifications
You must be signed in to change notification settings - Fork 197
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
enforce wheel size limits and README formatting in CI, put a ceiling on Cython dependency #2490
Conversation
Seeing build issues like this when building
Note that this is only happening in |
Pasting my message from Slack:
|
Thanks @vyasr ! It looks like adding the ceiling on Cython worked 🎉 Put up #2491 to track the work of relaxing that ceiling in the future. Some CI jobs here are failing for what look like temporary issues, but I don't want to stop the in-progress C++ test job to restart everything, because those take several hours to run 😬 I'll keep checking back and hopefully we can get a clean CI run and then merge this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question / one comment. Overall it should be fine.
dependencies.yaml
Outdated
@@ -164,7 +164,7 @@ dependencies: | |||
- output_types: [conda, requirements, pyproject] | |||
packages: | |||
- &cmake_ver cmake>=3.26.4,!=3.30.0 | |||
- cython>=3.0.0 | |||
- cython>=3.0.0,<=3.1.0a0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this supposed to be strictly less than? (This appears several places.)
- cython>=3.0.0,<=3.1.0a0 | |
- cython>=3.0.0,<3.1.0a0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahhh yes!!! Thank you for catching this 🙈
Updated all of them in 4206485
] | ||
|
||
# detect when package size grows significantly | ||
max_allowed_size_compressed = '825M' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, this size should decrease a lot with #2488.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🥳 hey awesome! We should decrease this in #2488 or some follow-up PR after.
/merge |
Following #2498, we can apply this feedback from #2490: #2490 (comment) These changes are inspired by rapidsai/cuvs#469. Authors: - Bradley Dice (https://github.com/bdice) Approvers: - Kyle Edwards (https://github.com/KyleFromNVIDIA) URL: #2509
Description
Contributes to rapidsai/build-planning#110
Proposes adding 2 types of validation on wheels in CI, to ensure we continue to produce wheels that are suitable for PyPI.
Also puts a ceiling on Cython to its latest stable release (
<=3.0.11
), to fix #2490 (comment). Work to relax that is tracked in (#2491).