-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
[ci] Clean wheels directory before build, validate wheel commit strings #19097
Conversation
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.
Thanks for fixing the issue.
Besides .whl/, is it possible that we have other leftover from previous build that might mess up with the current build? Should we consider buildkite Clean checkout
?
I think this is a good idea - cc @simon-mo do we have to add this to the buildkite ci stack? |
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.
looks good to me. cc @simon-mo for sanity check.
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.
LGTM
This might not work as .whl from linux actually comes from a shared component (docker-in-docker container) that's harder to clean. We are already doing clean checkout as a matter of fact. |
The good news: The sanity check works! https://buildkite.com/ray-project/ray-builders-pr/builds/15243#47a50ee0-841c-4e5e-a6fa-eb21c683b24e The bad news: Cleaning up the |
Why are these changes needed?
Buildkite runners are re-used and sometimes have leftover state.
This is illustrated in these two builds:
https://buildkite.com/ray-project/ray-builders-branch/builds/3798#98bd4bff-8b77-484d-94a9-5e09ea5f13e2
and
https://buildkite.com/ray-project/ray-builders-branch/builds/3799#283c057c-6334-466b-805d-75535a16e169
(click on "Timeline").
In this case, wheels from the first build (2.0.0.dev0) still exist on the runner and are uploaded again in the next build (1.7.0). In this case this is obvious, but sometimes this happens for the same version string and can lead to confusion.
This PR clears the
.whl
directory inci.sh build
so that previous build wheels are deleted. We also introduce a sanity check that extracts the wheel commit string from each wheel and compares it to the expected commit.Related issue number
Closes #18702
Checks
scripts/format.sh
to lint the changes in this PR.