refactor: switch to rapids-artifact-name for consistent artifact naming#2136
Open
gforsyth wants to merge 2 commits into
Open
refactor: switch to rapids-artifact-name for consistent artifact naming#2136gforsyth wants to merge 2 commits into
rapids-artifact-name for consistent artifact naming#2136gforsyth wants to merge 2 commits into
Conversation
gforsyth
added
improvement
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
Contributor
Author
|
/ok to test |
gforsyth
added
non-breaking
20 tasks
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughCI/CD scripts and workflows switch artifact naming to ChangesArtifact naming and download refactor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ci/test_python.sh (1)
13-14: ⚡ Quick winAdd explicit validation for downloaded channel values.
Line 13 and Line 14 should fail fast with a clear message if channel resolution returns empty output.
Proposed patch
CPP_CHANNEL=$(rapids-download-from-github "$(rapids-artifact-name conda_cpp libcuvs cuvs --cuda "$RAPIDS_CUDA_VERSION")") PYTHON_CHANNEL=$(rapids-download-from-github "$(rapids-artifact-name conda_python cuvs cuvs --stable --cuda "$RAPIDS_CUDA_VERSION")") + +if [[ -z "${CPP_CHANNEL}" || -z "${PYTHON_CHANNEL}" ]]; then + rapids-logger "ERROR: Failed to resolve one or more conda channels from GitHub artifacts." + exit 1 +fiAs per coding guidelines,
ci/**/*requires “proper error handling and meaningful error messages”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/test_python.sh` around lines 13 - 14, After assigning CPP_CHANNEL and PYTHON_CHANNEL in ci/test_python.sh (the results of rapids-download-from-github/rapids-artifact-name calls), add immediate validation: check if CPP_CHANNEL or PYTHON_CHANNEL is empty or unset right after their assignment, and if so print a clear error message identifying which channel failed to resolve and exit non‑zero; ensure the check references the exact variable names (CPP_CHANNEL, PYTHON_CHANNEL) and runs before any later use so the script fails fast with a meaningful message.ci/test_wheel_cuvs.sh (1)
12-13: ⚡ Quick winValidate wheelhouse paths immediately after artifact download.
Line 12 and Line 13 should assert that both wheelhouse directories exist and fail with a clear error before install.
Proposed patch
LIBCUVS_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_cpp libcuvs cuvs --cuda "$RAPIDS_CUDA_VERSION")") CUVS_WHEELHOUSE=$(rapids-download-from-github "$(rapids-artifact-name wheel_python cuvs cuvs --stable --cuda "$RAPIDS_CUDA_VERSION")") + +if [[ ! -d "${LIBCUVS_WHEELHOUSE}" || ! -d "${CUVS_WHEELHOUSE}" ]]; then + rapids-logger "ERROR: Wheelhouse download failed. LIBCUVS_WHEELHOUSE='${LIBCUVS_WHEELHOUSE}', CUVS_WHEELHOUSE='${CUVS_WHEELHOUSE}'" + exit 1 +fiAs per coding guidelines,
ci/**/*requires “proper error handling and meaningful error messages”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ci/test_wheel_cuvs.sh` around lines 12 - 13, After downloading artifacts into LIBCUVS_WHEELHOUSE and CUVS_WHEELHOUSE using rapids-download-from-github/rapids-artifact-name, immediately verify each path exists and is a non-empty directory; if a check fails, print a clear error mentioning the variable name (LIBCUVS_WHEELHOUSE or CUVS_WHEELHOUSE) and exit with a non-zero status. Update the shell snippet around the assignments to perform [ -d "$LIBCUVS_WHEELHOUSE" ] and [ -d "$CUVS_WHEELHOUSE" ] (and optionally check contents) and call exit 1 with descriptive messages so installs do not proceed when downloads failed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@ci/test_python.sh`:
- Around line 13-14: After assigning CPP_CHANNEL and PYTHON_CHANNEL in
ci/test_python.sh (the results of
rapids-download-from-github/rapids-artifact-name calls), add immediate
validation: check if CPP_CHANNEL or PYTHON_CHANNEL is empty or unset right after
their assignment, and if so print a clear error message identifying which
channel failed to resolve and exit non‑zero; ensure the check references the
exact variable names (CPP_CHANNEL, PYTHON_CHANNEL) and runs before any later use
so the script fails fast with a meaningful message.
In `@ci/test_wheel_cuvs.sh`:
- Around line 12-13: After downloading artifacts into LIBCUVS_WHEELHOUSE and
CUVS_WHEELHOUSE using rapids-download-from-github/rapids-artifact-name,
immediately verify each path exists and is a non-empty directory; if a check
fails, print a clear error mentioning the variable name (LIBCUVS_WHEELHOUSE or
CUVS_WHEELHOUSE) and exit with a non-zero status. Update the shell snippet
around the assignments to perform [ -d "$LIBCUVS_WHEELHOUSE" ] and [ -d
"$CUVS_WHEELHOUSE" ] (and optionally check contents) and call exit 1 with
descriptive messages so installs do not proceed when downloads failed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 5332f8f5-a8c9-4491-a14e-ddf0dc3b45f4
📒 Files selected for processing (12)
.github/workflows/pr.yamlci/build_cpp.shci/build_docs.shci/build_go.shci/build_java.shci/build_python.shci/build_rust.shci/build_wheel_cuvs.shci/build_wheel_libcuvs.shci/test_cpp.shci/test_python.shci/test_wheel_cuvs.sh
jameslamb
approved these changes
May 27, 2026
|
Actionable comments posted: 0 |
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
2 participants
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.
This PR swaps in
rapids-artifact-nameforrapids-package-nameeverywhere, and also removes any legacy named artifacts. All artifacts now follow the same naming convention (and that convention can be updated/expanded from a central location). Part of rapidsai/build-planning#270