-
Notifications
You must be signed in to change notification settings - Fork 519
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: limit downloaded files in build_docker #4242
ci: limit downloaded files in build_docker #4242
Conversation
This download-artifact step fails randomly. Limiting the files to download should reduce the possibility of failure. Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
📝 WalkthroughWalkthroughThe changes in the pull request involve modifications to the GitHub Actions workflow in Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .github/workflows/build_wheel.yml (2 hunks)
🧰 Additional context used
🔇 Additional comments (1)
.github/workflows/build_wheel.yml (1)
151-152
: LGTM! Verify the artifact pattern matches wheel naming convention.The pattern
cibw-*-manylinux_x86_64-cu${{ matrix.cuda_version }}*
effectively limits downloads to only the required wheels for each CUDA version. This should help reduce random failures in the download-artifact step.Let's verify the pattern matches the wheel naming convention:
✅ Verification successful
Let me try a different approach to verify the artifact naming pattern by examining the workflow file and related configuration.
From examining the workflow file, I can now verify the artifact naming pattern. Let me analyze the key findings:
Pattern matches wheel naming convention - Verified
The pattern
cibw-*-manylinux_x86_64-cu${{ matrix.cuda_version }}*
is correct because:
The workflow shows that artifacts are uploaded with matching names:
name: cibw-cp${{ matrix.python }}-${{ matrix.platform_id }}-cu${{ matrix.cuda_version }}-${{ strategy.job-index }}The matrix values align perfectly:
- CUDA versions are defined as "12.2" and "11.8" in the matrix
- Platform ID is explicitly set as "manylinux_x86_64"
- The pattern will correctly match these artifacts
The build_docker job's matrix correctly maps to these artifacts:
matrix: include: - variant: "" cuda_version: "12" - variant: "_cu11" cuda_version: "11"🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the wheel filenames match the download pattern # Expected: Wheel filenames should contain "manylinux_x86_64" and "cu{version}" # Test: List artifacts from a recent successful workflow run gh api \ -H "Accept: application/vnd.github+json" \ -H "X-GitHub-Api-Version: 2022-11-28" \ "/repos/deepmodeling/deepmd-kit/actions/artifacts" \ --jq '.artifacts[] | select(.name | contains("manylinux_x86_64")) | .name'Length of output: 211
Script:
#!/bin/bash # Check the workflow file for artifact naming patterns and CUDA matrix values echo "=== Checking workflow file ===" cat .github/workflows/build_wheel.yml # Check for any cibuildwheel config that might affect naming echo -e "\n=== Checking for cibuildwheel config ===" fd "setup.cfg|pyproject.toml" # Look for any test or build scripts that might show artifact naming echo -e "\n=== Checking for build scripts ===" fd -t f "build|test" -e sh -e pyLength of output: 18018
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #4242 +/- ##
=======================================
Coverage 84.55% 84.55%
=======================================
Files 537 537
Lines 51237 51237
Branches 3047 3047
=======================================
Hits 43324 43324
Misses 6965 6965
Partials 948 948 ☔ View full report in Codecov by Sentry. |
This download-artifact step fails randomly. Limiting the files to download should reduce the possibility of failure. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced artifact downloading patterns for improved specificity in the build process. - Expanded artifact access during the PyPI index build. - **Improvements** - Maintained job structure and dependencies for consistent build and deployment processes. - Defined permissions for critical jobs to ensure secure operations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu> (cherry picked from commit 8e77785)
This download-artifact step fails randomly. Limiting the files to download should reduce the possibility of failure. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced artifact downloading patterns for improved specificity in the build process. - Expanded artifact access during the PyPI index build. - **Improvements** - Maintained job structure and dependencies for consistent build and deployment processes. - Defined permissions for critical jobs to ensure secure operations. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu> (cherry picked from commit 8e77785)
This download-artifact step fails randomly. Limiting the files to download should reduce the possibility of failure.
Summary by CodeRabbit
New Features
Improvements