[CI] Update workflow #473
[CI] Update workflow #473
Conversation
|
Warning Rate limit exceeded@zhiyuan1i has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 43 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (6)
WalkthroughThe GitHub Actions workflows for various GPU runners were updated to split tests into two jobs: one for operational tests excluding models, and another for model-specific tests. The Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant test-ops job
participant test-models job
participant find_dependent_tests.py
GitHub Actions->>test-ops job: Start (on PR/tag)
test-ops job->>find_dependent_tests.py: Run with TEST_SCOPE=EXCLUDE_MODELS
find_dependent_tests.py-->>test-ops job: Return non-model test files
test-ops job->>test-ops job: Run pytest on selected tests
GitHub Actions->>test-models job: Start (after test-ops)
test-models job->>find_dependent_tests.py: Run with TEST_SCOPE=MODELS_ONLY
find_dependent_tests.py-->>test-models job: Return model test files
test-models job->>test-models job: Run pytest on selected tests
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/nvidia-4090.yml (1)
125-125: Static analysis warning about custom runner labels.Same actionlint warning as in nvidia-h100 workflow for self-hosted runner labels.
Refer to the previous comment about configuring actionlint for custom runner labels.
🧹 Nitpick comments (5)
.github/workflows/intel-a770.yml (1)
59-59: Consider adding a test-models job for consistency.While the
TEST_SCOPE=EXCLUDE_MODELSaddition is correct, this workflow lacks thetest-modelsjob that other GPU workflows (nvidia-h100, nvidia-4090) have. This creates inconsistency in test coverage across different environments.Consider adding a
test-modelsjob similar to other GPU workflows to ensure model tests are executed on Intel hardware as well..github/workflows/nvidia-a100.yml (4)
59-61: Optimize test discovery invocation
Consider definingTEST_SCOPEas a job-levelenvor quoting${{ steps.changed-files.outputs.all_changed_files }}to guard against paths with whitespace.
62-75: Use explicit Python setup and cache dependencies
Rather than relying on the runner’s default Python interpreter and reinstalling packages on every run, useactions/setup-python@v4to pin a Python version andactions/cacheto cache~/.cache/pip. This will improve build stability and speed.
126-126: Unknown self-hosted runner label
runs-on: 'nvidia-a100'isn’t a standard GitHub-hosted label. If this is a custom self-hosted runner, addnvidia-a100to your Actionlint configuration to silence label warnings.
124-229: DRY up duplicated jobs
The newtest-modelsjob duplicates nearly all steps fromtestexcept for theTEST_SCOPEargument. Extract the common sequence (checkout, skip logic, change detection, env/bootstrap, GPU check, pytest invocations) into a reusable workflow or composite action to reduce maintenance overhead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.github/workflows/intel-a770.yml(3 hunks).github/workflows/lint.yaml(1 hunks).github/workflows/nvidia-4090.yml(3 hunks).github/workflows/nvidia-a100.yml(2 hunks).github/workflows/nvidia-h100.yml(3 hunks)fla/ops/rwkv7/fused_k_update.py(1 hunks)scripts/find_dependent_tests.py(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: lint
fla/ops/rwkv7/fused_k_update.py
[error] 11-11: flake8: E302 expected 2 blank lines, found 1
🪛 actionlint (1.7.7)
.github/workflows/nvidia-h100.yml
126-126: label "nvidia-h100" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/nvidia-4090.yml
125-125: label "nvidia-4090" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/nvidia-a100.yml
126-126: label "nvidia-a100" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test-models
🔇 Additional comments (15)
.github/workflows/lint.yaml (1)
30-30: Bumptj-actions/changed-filesto v46.0.5 for consistency
This update aligns with other GPU workflows and ensures consistent changed-file detection. No issues spotted.scripts/find_dependent_tests.py (2)
2-2: LGTM: Clean import addition for environment variable support.The
osimport is correctly added to support the newTEST_SCOPEenvironment variable functionality.
45-58: LGTM: Well-implemented test scoping logic.The test filtering implementation correctly handles all three scopes:
MODELS_ONLY: Includes only tests undertests/models/EXCLUDE_MODELS: Excludes model tests but includes all othersALL: Default behavior (all tests)The path logic using
models_test_dir not in p.parentsfor exclusion is correct and efficient..github/workflows/intel-a770.yml (2)
18-18: LGTM: Job renamed to reflect test scope.The job name change from
testtotest-opsclearly indicates this job now handles non-model tests.
49-49: LGTM: Action version updated.Updated to the newer version of
tj-actions/changed-filesfor consistency across workflows..github/workflows/nvidia-h100.yml (6)
18-18: LGTM: Clear job naming for test scoping.The job rename to
test-opsclearly indicates this handles non-model tests.
49-49: LGTM: Action version updated for consistency.Updated to the same version as other workflows.
59-61: LGTM: Proper test scoping implementation.The
TEST_SCOPE=EXCLUDE_MODELScorrectly filters out model tests for this job.
62-76: LGTM: Comprehensive Python environment setup.The setup includes proper dependency management with
uv, specific versions, and necessary packages likeflash-attnandcausal-conv1d.
124-166: LGTM: Well-structured test-models job.The new job properly handles model-specific tests with
TEST_SCOPE=MODELS_ONLYand maintains consistency with the test-ops job structure.
126-126: Address static analysis warning about custom runner labels.The actionlint warning about unknown runner label
nvidia-h100is expected for self-hosted runners.To suppress this warning, consider adding an
actionlint.yamlconfig file at the repository root:self-hosted-runner: labels: - nvidia-h100 - nvidia-4090 - nvidia-a100 - intel-a770How to configure actionlint for self-hosted GitHub Actions runners with custom labels?.github/workflows/nvidia-4090.yml (3)
49-49: LGTM: Action version updated for consistency.Updated to match other workflows.
59-59: LGTM: Proper test scoping for existing test job.The addition of
TEST_SCOPE=EXCLUDE_MODELScorrectly excludes model tests from the main test job.
123-165: LGTM: Consistent test-models job implementation.The new job structure is consistent with the nvidia-h100 workflow, ensuring maintainability across GPU environments.
.github/workflows/nvidia-a100.yml (1)
49-49: Verify action version bump
Ensuretj-actions/[email protected]is available and compatible across all GPU workflows.
zhiyuan1i
force-pushed
the
ci-pipeline
branch
2 times, most recently
from
360043c to
8b3e727
Compare
1 participant
Summary by CodeRabbit