Skip to content
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

Run wheel build on fewer PRs? #3879

Closed
JelleZijlstra opened this issue Sep 11, 2023 · 3 comments · Fixed by #3881
Closed

Run wheel build on fewer PRs? #3879

JelleZijlstra opened this issue Sep 11, 2023 · 3 comments · Fixed by #3881

Comments

@JelleZijlstra
Copy link
Collaborator

As of #3870 we now run the full mypyc wheel build workflow on all PRs. This should prevent a repeat of the 23.9.0 release, which didn't come with mypyc wheels because after the release was cut we discovered that the mypyc build was broken (#3865).

It's very important to me that we can test the wheel build before the release, as is happening right now on #3878 for the upcoming release 23.9.1. That gives me confidence to cut a release knowing that the build will actually succeed.

Some of the issues with the current wheel build were caused by #3821. Ideally, we would have noticed these issues before that PR was merged, not when we tried to cut a release that included it.

However, the full wheel build is pretty slow and it may not be all that useful on most PRs, so running the full workflow may end up slowing things down too much. Here's a few non-exclusive ideas that could help:

  • We could trigger the workflow only for PRs with a particular label. We would add that label to release PRs, and also to bigger changes like Improve caching by comparing file hashes as fallback for mtime and size #3821 to confirm they don't break mypyc.
  • Running only the Linux wheel build (which tends to be the fastest) is often good enough, as most mypyc issues don't tend to be OS-specific. So maybe for low-risk PRs we could just run the Linux workflow.
  • We could run the workflow at scheduled times (e.g. once a week). Not sure how much that would help though, as the wheel build should ideally depend just on what's in the repo (e.g., we pin the mypy version).

Happy to hear any other ideas too.

@hauntsaninja
Copy link
Collaborator

We can build on pushes to main and maybe just have cibuildwheel build a single configuration on PRs

@cdce8p
Copy link
Contributor

cdce8p commented Sep 11, 2023

We can build on pushes to main and maybe just have cibuildwheel build a single configuration on PRs

A single configuration might not be enough. In particular #3821 was only an issue with Python 3.8. For PRs maybe the lowest and highest supported versions? I.e. cp38 and cp311. Additionally it would probably be helpful to have a job for cp312 as well, which is allowed to fail.

Running those for Linux is likely enough though.

@JelleZijlstra
Copy link
Collaborator Author

I'm not a huge fan of testing only on pushes to main, because that still means we only learn of an issue after a PR has been merged. It's still better than learning about it only after the release has been cut, but I prefer if we can see mypyc troubles before a PR is merged.

Agree that we should test multiple Python versions, as mypyc does often tend to behave differently on different versions.

Maybe we run cp38 and cp311 on all PRs, and add a label that triggers all of the workflows?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants