Repository navigation
[WoA] Add manual cross-repository broker - #3025
Conversation
|
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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 Summary
Priority: ⬇️ Low Change: Feature Merge Risk: 🟡 Moderate · up to
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/woa-private-dispatch.yml (1)
67-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Derive the CUDA build version from
ci/versions.ymlatpublic_sha. Do not hard-code13.4.2.Two values are fixed to
13.4.2:
- the producer job name
Build win-arm64, CUDA 13.4.2 / py3.13;- the artifact name
cuda-bindings-python313-cuda13.4.2-win-arm64-<sha>.In
.github/workflows/ci.yml, both values come from.cuda.build.version(Lines 224-242 and 444-467). After the next CUDA version bump, this broker will reject every valid public run until someone edits this file. The failure is safe, but the broker becomes unusable.Fix: read
ci/versions.ymlatpublic_shawithgh api repos/NVIDIA/cuda-python/contents/ci/versions.yml?ref=$public_sha. Then build the job name and the artifact names from.cuda.build.version.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
3bdf7831-9285-4563-bf65-3d0b14907b12
📒 Files selected for processing (2)
.github/workflows/woa-crossrepo-smoke.yml.github/workflows/woa-private-dispatch.yml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
|
|
The PR needs manual trigger to run smoke test and dispatch-validate test, if I understand correctly. Are we able to test the manual trigger when the PR is open, or we need to merge the PR to allow manual trigger? |
|
The PR introduces a two-phase design: a lightweight smoke check between the public and private repos to cheaply validate cross-repo connectivity/config, followed by a dispatch-validate step that verifies a merged main build and triggers real GB10 run. This lets connectivity/config issues surface cheaply in the first phase, before the costlier validation phase. LGTM. I just have a few questions. |
There's a way, as pointed out here: https://github.com/NVIDIA-dev/cuda-python-private/pull/615#issuecomment-5797975839 But there's still a gap: |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
f257626e-4b8d-488c-bad5-26dc8c1e75d6
📒 Files selected for processing (2)
.github/workflows/woa-transport-check.yml.github/workflows/woa-validation-dispatch.yml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
lijinf2
left a comment
lijinf2
left a comment
There was a problem hiding this comment.
LGTM.
.path mismatched issue raised by coderabbit seems not blocking merge. We can confirm whether that is an actual issue after merge.
So we will have two new rows under Github Action. One is "woa-transport-check", and the other is "woa-validation-dispath". Does "woa-validation-dispath" changes status (i.e. green or red) before the private run finished or after?
The check run will get updated depend on the stage of the private run. For example, after the dispatch is admitted, the private run will create a POST to update the check with "in progress" status. And once test finished, the report job will update the check with |
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| public_run_id: | ||
| description: Successful public main CI run containing the exact WoA artifacts | ||
| required: true | ||
| type: string |
There was a problem hiding this comment.
nit: For convenience we can make this optional. We have a lookup-run-id script that can look up the latest run ID, see ci-nightly.yml:
cuda-python/.github/workflows/ci-nightly.yml
Lines 25 to 32 in 5fe0749
| jq -e ' | ||
| .repository.id == 381173759 and | ||
| .workflow_id == 155304118 and | ||
| .path == ".github/workflows/ci.yml" and | ||
| .event == "push" and | ||
| .head_branch == "main" and | ||
| .status == "completed" and | ||
| .conclusion == "success" and | ||
| (.head_sha | test("^[0-9a-f]{40}$")) | ||
| ' <<< "$run" >/dev/null | ||
| public_sha=$(jq -r .head_sha <<< "$run") |
There was a problem hiding this comment.
ditto, this can be retrieved by passing --head-sha to lookup-run-id
| artifacts-json: ${{ steps.resolve.outputs.artifacts_json }} | ||
| correlation-id: ${{ steps.resolve.outputs.correlation_id }} | ||
| public-producer-job-id: ${{ steps.resolve.outputs.public_producer_job_id }} | ||
| public-run-attempt: ${{ steps.resolve.outputs.public_run_attempt }} |
There was a problem hiding this comment.
We have enough sophisticated artifact passing across pipelines, but so far I don't see any need for run_attempt (because multiple attempts share the same run ID). Is it really needed?
There was a problem hiding this comment.
(ditto for public_producer_job_id)
There was a problem hiding this comment.
We don't - to download the artifact we can just use gh run download as suggested.
| expected_names=$(jq -n \ | ||
| --arg sha "$public_sha" '[ | ||
| "cuda-pathfinder-wheel", | ||
| ("cuda-bindings-python313-cuda13.4.2-win-arm64-" + $sha), | ||
| ("cuda-core-python313-win-arm64-" + $sha) | ||
| ]') | ||
| artifacts_json='[]' | ||
| while IFS= read -r artifact_name; do | ||
| response=$(gh api --method GET \ | ||
| -H 'X-GitHub-Api-Version: 2026-03-10' \ | ||
| "repos/NVIDIA/cuda-python/actions/runs/$PUBLIC_RUN_ID/artifacts" \ | ||
| -f name="$artifact_name" \ | ||
| -F per_page=100) | ||
| artifact=$(jq -c -e \ | ||
| --arg name "$artifact_name" ' | ||
| if .total_count == 1 and | ||
| (.artifacts | length) == 1 and | ||
| .artifacts[0].name == $name and | ||
| .artifacts[0].expired == false | ||
| then .artifacts[0] | {id, name, digest} | ||
| else error("expected exactly one unexpired artifact named " + $name) | ||
| end | ||
| ' <<< "$response") | ||
| artifacts_json=$(jq -c \ | ||
| --argjson artifact "$artifact" \ | ||
| '. + [$artifact] | sort_by(.name)' <<< "$artifacts_json") | ||
| done < <(jq -r '.[]' <<< "$expected_names") | ||
| jq -e ' | ||
| length == 3 and | ||
| all(.[ ]; | ||
| (.id | type == "number") and | ||
| (.digest | test("^sha256:[0-9a-f]{64}$"))) | ||
| ' <<< "$artifacts_json" >/dev/null |
There was a problem hiding this comment.
Q: I am a bit confused by this code. Is artifacts_json a hard requirement? I think providing just the repo name and the run ID is enough to retrieve artifacts, akin to gh run download ...? All we need on the private repo side is a GH token that has actions:read.
There was a problem hiding this comment.
It's not. We only need public_run_id with gh run download to download the artifacts. gh run download also support filtering via name or glob flags.
|
@isVoid the comments above also apply to the private counterpart, so I don't plan to repeat what's stated here. Feel free to merge and create a tracking issue to follow up. I did not block the merge. |
|
FYI, when reviewing the pair of PRs agents just gave a thumbs up without asking all the questions I asked above. Need to bang them particularly hard for them to see a path toward simplification. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
90ce3533-9e8a-4b8b-95c3-e62547f84c2a
📒 Files selected for processing: 1
.github/workflows/woa-validation-dispatch.yml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 10 remain after this review.
| set -euo pipefail | ||
| public_sha='' | ||
| if [[ -z "$PUBLIC_RUN_ID" ]]; then | ||
| lookup_output=$(./ci/tools/lookup-run-id --branch main --head-sha NVIDIA/cuda-python CI) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
important: Select a successful push run when the run ID is empty. lookup-run-id can select a successful scheduled or manually dispatched main CI run. The later .event == "push" check then rejects that run without trying an older push run. CI supports both event types, including a daily schedule. Filter candidates by push during lookup so the empty-input path can resolve a qualifying run. (raw.githubusercontent.com)
There was a problem hiding this comment.
In the follow up PR #3026, I will update lookup-run-id to include a push-event filter.
4 participants
Description
Adds two manually triggered workflows for the initial cross-repository WoA CI pilot. This PR enables no automatic validation on pushes, schedules, or pull requests.
Paired private implementation: https://github.com/NVIDIA-dev/cuda-python-private/pull/650. Tracks https://github.com/NVIDIA-dev/cuda-python-private/issues/584.
Transport check
woa-transport-check.yml(WoA transport check) takes no inputs. It resolves the current publicmainSHA, verifies its ancestry, and dispatchesctk-next-woa-transport-check.ymlon privatectk-next. The private workflow independently verifies the SHA and createscuda-python WoA integration / transport smokeon that public commit, transitioning fromin_progresstocompleted: neutral.The resolved SHA stays fixed if
mainadvances. This checks the App credential exchange and public reporting without downloading wheels or allocating GB10. It does not target a draft PR or an unmerged PR head.Manual wheel-validation dispatch
woa-validation-dispatch.yml(WoA validation dispatch) accepts an optionalpublic_run_id:mainCI run.ci/tools/lookup-run-id --branch main --head-shahelper.The public resolver checks the repository name, CI workflow identity/path,
pushevent,mainbranch, successful completion, SHA, and ancestry before dispatching the exact run ID and SHA privately. Repository numeric IDs, producer-job IDs, run attempts, and artifact manifests are not passed.Private admission discovers the three expected artifacts within that run, validates their metadata and archive digests, stages the wheels, runs a Python 3.13 install/import smoke on GB10, and reports the result through
cuda-python WoA integrationon the selected public SHA. The public broker completes once dispatch is verified; its own success does not mean GB10 testing has finished. The separate App Check Run carries that result.The private-dispatch App PEM remains a repository secret under the agreed trusted-writer model. Its token is scoped to private
Actions: write; GitHub grants that permission across private Actions, rather than restricting it to these two workflows. Only isolated dispatch jobs mint/use this token; those jobs perform no checkout or artifact processing.Rollout
ctk-next, then merge this PR into publicmain.WoA transport checkfrommain, with no inputs. Inspect the current target commit's checks forcuda-python WoA integration / transport smokecompleting asneutral. No GB10 job should start.WoA validation dispatchfrommainwith a recent successful push-triggered publicCIrun ID whose WoA artifacts are still available. Inspect the canonical public Check Run and the private GB10 workflow. Its smoke installs/imports three wheels; it is not the full GPU test suite.Pilot limits and follow-up
mainCI run regardless of event. A scheduled/manual candidate is then rejected by thepushassertion without trying an older push run. Supplying an eligible run ID is the current workaround; an event filter forlookup-run-idis deferred to [WoA] Automate public cross-repository dispatch #3026.Validation
mainSHA.38473329258and its three expected artifacts. The current PR workflow files match the simulated versions.