reduce duplication in rapids-get-pr-* scripts - #173
Merged
Conversation
3 tasks
This was referenced May 13, 2025
jameslamb
marked this pull request as ready for review
May 27, 2025 17:02
bdice
approved these changes
May 27, 2025
bdice
left a comment
Contributor
There was a problem hiding this comment.
Nice! All this makes sense. Thanks especially for your writeup on "Is this safe to merge?" because I had to think about that for a bit.
ajschmidt8
approved these changes
May 27, 2025
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.
Contributes to https://github.com/rapidsai/build-infra/issues/237
For a long time now, we've had tools in this project to make it easier to use one project's CI artifacts in another project's CI:
rapids-get-pr-conda-artifactrapids-get-pr-wheel-artifactToday, those download files from the
downloads.rapids.aiS3 bucket. So when the GitHub Actions artifact storage stuff was first rolling out, #152 introduced GitHub-y versions of those:rapids-get-pr-conda-artifact-githubrapids-get-pr-wheel-artifact-githubNow that all RAPIDS projects are uploading conda packages and wheels to the GitHub Actions artifact store, it's safe to remove those
*-githubscripts and just change the behavior of the others to use the GitHub Actions artifact store instead of S3.Notes for Reviewers
Is it safe to merge this?
Yes. These scripts are not used in any long-lived CI.
GitHub search: https://github.com/search?q=%2Frapids%5C-get%5C-pr%2F%20AND%20NOT%20is%3Aarchived&type=code
They're only used temporarily on PRs during testing. See the docs where that's described: https://github.com/rapidsai/docs/blob/00878de8b9afead0f8d44221c228fda2dbbbd97e/resources/github-actions.md?plain=1#L186-L188
How I tested this
Created an
rmmPR using this branch'sgha-tools: rapidsai/rmm#1909And then a
cudfPR using this branch'sgha-toolsand the packages produced by CI from thatrmmPR: NVIDIA/cudf#18747Checked logs to confirm that the downloaded
rmmpackages were used:branch-25.06testing: WIP: [DO NOT MERGE] test gha-tools changes NVIDIA/cudf#18747 (comment)branch-25.08testing: WIP: [DO NOT MERGE] test gha-tools changes NVIDIA/cudf#18747 (comment)