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

do not install cudf in cudf_polars wheel tests #16612

Merged
merged 2 commits into from
Aug 20, 2024

Conversation

jameslamb
Copy link
Member

Description

Removes unnecessary installation of cudf wheels in wheel testing for cudf_polars.

cudf_polars doesn't depend on cudf, and neither do its tests. However, right now it's downloading cudf during it's wheel tests. I mistakenly introduced that in #16575.

This introduced a race condition that could lead to CI failures whenever the cudf wheels aren't published yet by the time the cudf_polars tests. Because the cudf_polars wheel tests (rightly) do not wait for cudf wheels to be available:

wheel-tests-cudf-polars:
needs: wheel-build-cudf-polars

wheel-build-cudf-polars:
needs: wheel-build-pylibcudf

Noticed this in #16611

[rapids-download-from-s3] Downloading and decompressing s3://rapids-downloads/ci/cudf/pull-request/16611/a6b7eff/cudf_wheel_python_cudf_cu12_py310_x86_64.tar.gz into ./dist
download failed: s3://rapids-downloads/ci/cudf/pull-request/16611/a6b7eff/cudf_wheel_python_cudf_cu12_py310_x86_64.tar.gz to - An error occurred (404) when calling the HeadObject operation: Not Found

(build link)

Checklist

  • I am familiar with the Contributing Guidelines.
  • New or existing tests cover these changes.
  • The documentation is up to date with these changes.

@jameslamb jameslamb added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Aug 20, 2024
@jameslamb jameslamb requested a review from a team as a code owner August 20, 2024 14:52
@jameslamb jameslamb requested review from AyodeAwe, vyasr and bdice August 20, 2024 14:52
@jameslamb
Copy link
Member Author

/merge

@vyasr
Copy link
Contributor

vyasr commented Aug 20, 2024

This wasn't your fault. There's some confusion here. The initial version cudf-polars that we will first release does depend on cudf, but that's going to be based on 24.08. In 24.10 we now have a pylibcudf package and that's the only dependency for cudf-polars.

@rapids-bot rapids-bot bot merged commit b32bc10 into rapidsai:branch-24.10 Aug 20, 2024
79 of 81 checks passed
@jameslamb jameslamb deleted the ci-order branch August 20, 2024 18:16
@jameslamb
Copy link
Member Author

Ah ok, thanks for that @vyasr ! I didn't realize that, that's helpful.

@jameslamb jameslamb mentioned this pull request Aug 21, 2024
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants