Repository navigation
fix(ci): resolve runners with the therock-ci-config v2 API - #12960
Merged
Merged
Conversation
therock-ci-config dropped Config.get_gpu_families() on 30 Sep (aad3ef6985), so every resolve fell back to the hardcoded labels. For gfx90a that is linux-gfx90a-gpu-rocm (smci250) instead of linux-gfx90a-1gpu-ossci-rocm. Read get_gpu_runner_labels() instead.
The mock still served get_gpu_families(trigger_types), which therock-ci-config dropped on 30 Sep, so the suite kept passing against an API the resolver no longer calls. The mock now mirrors v2: runner-config-v2.json keyed by family, and get_gpu_runner_labels(). TRIGGER_TYPES had no user left and goes with it. With the resolver switched back to get_gpu_families, 5 of the 10 tests fail.
andraghetti
requested review from
geomin12 and
venkat1361
and
a balanced review from Copilot
October 6, 2026 21:17
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation matches the upstream v2 API and updates the relevant test coverage consistently.
Review effort: Balanced
Findings: None
What changed in this PR
Updates CI runner resolution to use the therock-ci-config v2 API.
Changes:
- Replaces the removed v1 GPU-family getter with
get_gpu_runner_labels(). - Updates unit-test fixtures and mocks to reflect the v2 schema.
| File | Description |
|---|---|
resolve_ci_config.py |
Resolves GPU runner labels through the v2 API. |
test_resolve_ci_config.py |
Tests runner resolution using v2 configuration fixtures. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
venkat1361
approved these changes
Oct 6, 2026
1 task done
Contributor
TheRock Submodule Bump ActivityNewest first
|
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.
Motivation
Since 30 Sep, every workflow that uses
rocprofiler-sdk-util/resolve-ci-confighas been running on the fallback runner labels. Thetherock-ci-configlabels have not been used at all.therock-ci-config removed the v1 config on 30 Sep (ROCm/therock-ci-config#48). #12474 switched the resolver to
load_config(version=2). The next line still callsconfig.get_gpu_families(TRIGGER_TYPES), and v2 has no such method. The resolver catches theAttributeErrorand prints one warning per run:The unit tests did not catch it because their mock of
ci_config_apistill served the v1 method.Technical Details
resolve_ci_config.pyreadsconfig.get_gpu_runner_labels(). This is the v2 getter, already keyed by family and then by platform, so the rest of the function is unchanged.TRIGGER_TYPESno longer has a user and is removed.test_resolve_ci_config.pynow mocks the v2 API:runner-config-v2.jsonkeyed by family, andget_gpu_runner_labels().Against today's therock-ci-config
main, two outputs change from their fallback values:gfx90a_runnerlinux-gfx90a-gpu-rocmlinux-gfx90a-1gpu-ossci-rocmgfx94x_sandbox_runnerlinux-mi325-gpu-rocm-cpu-sandboxlinux-gfx942-8gpu-asan-sandbox-rocmAll the other outputs are the same as the fallbacks. The sandbox change comes from ROCm/therock-ci-config#46. It moves the rocprofiler-sdk sanitizer jobs to the 8-GPU ASAN sandbox.
Issue Tracking
None.
Test Plan
python -m unittest test_resolve_ci_config -v.get_gpu_families(...)line restored, the same tests were run again.resolve_ci_config.pyagainstci_config_api.pyandrunner-config-v2.jsonfrom therock-ci-configmain, with this branch and withdevelop.Test Result
developprints the warning and uses the fallbacks.Submission Checklist