Repository navigation
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
cloud-fan
left a comment
There was a problem hiding this comment.
Review summary
Deleting notify_test_workflow.yml is the only runtime change, and every hunk in update_build_status.yml changes only comments. For PRs into master, the scheduled updater's existing backfill now creates the Build check. It writes the same {owner, repo, run_id} run reference the notifier wrote, so checks the notifier already created keep syncing.
This review found no defects. The rewritten comments that still list reopened PRs as a source of duplicate Build checks stay accurate for now: PRs reopened before this change still carry the notifier's extra check, and the notifier copies on the maintenance branches keep adding one on each reopen until the backports land or ASF disables the trigger.
One optional follow-up suggestion for the author comes next, about the status a backfilled check starts with.
Findings
0 total: 0 P0, 0 P1, 0 P2, 0 P3.
No findings.
Re-review status
Prior AI findings: 0 addressed, 0 still present; additional unresolved findings in this review: 0.
New attribution: 0 newly introduced, 0 late catch, 0 previously raised, 0 unattributed.
Remaining prior AI findings
No prior AI findings remain.
Decision challenges
Optional follow-up: create the backfilled Build check with the fork run's current status instead of always queued
With the notifier gone, the backfill in update_build_status.yml creates every new Build check for PRs into master. It always creates the check as queued (lines 366-386), even though it already holds matched_run, the fork run for the head SHA, including its status and conclusion. The sync loop only walks the check runs it listed before the backfill (line 157), so a new check shows the fork's real state only on the next pass.
Compared with master:
- Fork run already finished when the check is created, for example when a contributor waits for fork CI before opening the PR. On master, the notifier creates the check within seconds and the next pass (0-15 minutes later) sets the result. With this PR, the first pass creates a
queuedcheck, and the result arrives only on the following pass, about 15 minutes later, or more if GitHub delays the schedule. - Fork run still in progress: only the intermediate label lags. The check shows
queuedinstead ofin_progressfor one extra pass, and the final result still arrives on the first pass after the run completes, the same as on master.
The check appearing up to one interval after a push follows from the schedule-only design, and the PR description accepts that. The extra interval on top of it does not. The backfill could create the check with matched_run's status and, when it is completed, its conclusion, mapped the same way the sync loop maps them (lines 220-252, including its fallbacks for values the check-runs API rejects), possibly through a helper shared with that loop. A PR opened after fork CI finished would then get its result as quickly as on master, with no new trigger or live-update mechanism.
This is non-blocking, and I think this PR is fine to merge as is, with update_build_status.yml changed only in comments. Keeping it comment-only is what lets the PR cite the updater's current production runs as its test, since the scheduled updater already runs from master, and one extra interval of queued is a small cost. Setting the initial status and conclusion at creation would make a reasonable optional follow-up; it is not a request for this PR.
|
Post-merge TODOs:
I'm going to merge this in and monitor the outcome for the next few hours. |
|
Merge Summary:
Posted by |
|
Post merge observations so far:
I will wait until tomorrow before backporting this to all branches going back to |
…st_target` trigger ### What changes were proposed in this pull request? Remove `notify_test_workflow.yml` and rely on the existing `update_build_status.yml` to report fork build status on PRs. Update the comments accordingly. This builds on [#58579](#58579) (SPARK-59293), which fixes errors that abort the scheduled scan. ### Why are the changes needed? ASF is disabling the `pull_request_target` trigger in [November 2026][asf], ahead of GitHub's [December 2 deadline][github]. This is our only workflow using it. The scheduled updater already creates missing `Build` checks (SPARK-58660), so we don't need another workflow to replace the notifier. Fork builds still run on push, as usual; this PR only changes how their status gets reported. We should backport this PR to active maintenance branches that accept PRs, since `pull_request_target` uses the base branch's workflow. [asf]: https://infra.apache.org/github-actions-policy.html [github]: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target#default-policy-for-pull_request_target I intentionally kept this PR simple as I _really_ don't like that we have to maintain so much GitHub Actions-specific JavaScript just to manage our fork-based contribution workflow. In other words, in the interest of simplicity I intentionally left out: - Any mechanism to update PRs "live". - Testing infrastructure for these workflows and the JavaScript they contain. This infrastructure is not in the Spark project's area of expertise. It really feels like something we should be able to import and use from somewhere. Until then, I think we should strive to keep these workflows as simple as possible. ### Does this PR introduce _any_ user-facing change? No, nothing for end users. For contributors here on GitHub: - The "Notify test workflow" check goes away. - New `Build` checks appear as `queued` on the first scheduled pass and pick up the fork result on the next. The schedule is every 15 minutes, but GitHub can delay or drop runs. - If you push multiple updates to a PR between scans, only the latest commit gets a `Build` check. - The results link opens the fork's Actions run instead of its `Run / Check changes` check. ### How was this patch tested? Checked locally with actionlint, YAML parsing, and git diff --check. `update_build_status.yml` changes only comments, so the scheduled updater, which runs from master, already behaves as it will after this PR. In [run 40517915808](https://github.com/apache/spark/actions/runs/40517915808) it created Build checks for PRs that had none (#54953, #59831), and later scheduled runs synced them (#54953 is now success). Additionally, an agent replayed the unmodified updater against 150 open PRs with their Build checks hidden and writes intercepted: all 150 resolve to the same fork run, and the 36 action_required PRs would get identical checks. All 708 open master PRs currently have exactly one Build check, and 706 match their fork run. This PR can't fully test itself because `pull_request_target` runs the base branch's copy of the notifier. I will run additional tests post merge. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Copilot SDK in VS Code (version unavailable) Closes #59829 from nchammas/pr-target-actions-policy. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: Nicholas Chammas <[email protected]> (cherry picked from commit 6a02fc4) Signed-off-by: Nicholas Chammas <[email protected]>
…st_target` trigger ### What changes were proposed in this pull request? Remove `notify_test_workflow.yml` and rely on the existing `update_build_status.yml` to report fork build status on PRs. Update the comments accordingly. This builds on [#58579](#58579) (SPARK-59293), which fixes errors that abort the scheduled scan. ### Why are the changes needed? ASF is disabling the `pull_request_target` trigger in [November 2026][asf], ahead of GitHub's [December 2 deadline][github]. This is our only workflow using it. The scheduled updater already creates missing `Build` checks (SPARK-58660), so we don't need another workflow to replace the notifier. Fork builds still run on push, as usual; this PR only changes how their status gets reported. We should backport this PR to active maintenance branches that accept PRs, since `pull_request_target` uses the base branch's workflow. [asf]: https://infra.apache.org/github-actions-policy.html [github]: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target#default-policy-for-pull_request_target I intentionally kept this PR simple as I _really_ don't like that we have to maintain so much GitHub Actions-specific JavaScript just to manage our fork-based contribution workflow. In other words, in the interest of simplicity I intentionally left out: - Any mechanism to update PRs "live". - Testing infrastructure for these workflows and the JavaScript they contain. This infrastructure is not in the Spark project's area of expertise. It really feels like something we should be able to import and use from somewhere. Until then, I think we should strive to keep these workflows as simple as possible. ### Does this PR introduce _any_ user-facing change? No, nothing for end users. For contributors here on GitHub: - The "Notify test workflow" check goes away. - New `Build` checks appear as `queued` on the first scheduled pass and pick up the fork result on the next. The schedule is every 15 minutes, but GitHub can delay or drop runs. - If you push multiple updates to a PR between scans, only the latest commit gets a `Build` check. - The results link opens the fork's Actions run instead of its `Run / Check changes` check. ### How was this patch tested? Checked locally with actionlint, YAML parsing, and git diff --check. `update_build_status.yml` changes only comments, so the scheduled updater, which runs from master, already behaves as it will after this PR. In [run 40517915808](https://github.com/apache/spark/actions/runs/40517915808) it created Build checks for PRs that had none (#54953, #59831), and later scheduled runs synced them (#54953 is now success). Additionally, an agent replayed the unmodified updater against 150 open PRs with their Build checks hidden and writes intercepted: all 150 resolve to the same fork run, and the 36 action_required PRs would get identical checks. All 708 open master PRs currently have exactly one Build check, and 706 match their fork run. This PR can't fully test itself because `pull_request_target` runs the base branch's copy of the notifier. I will run additional tests post merge. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Copilot SDK in VS Code (version unavailable) Closes #59829 from nchammas/pr-target-actions-policy. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: Nicholas Chammas <[email protected]> (cherry picked from commit 6a02fc4) Signed-off-by: Nicholas Chammas <[email protected]>
…st_target` trigger ### What changes were proposed in this pull request? Remove `notify_test_workflow.yml` and rely on the existing `update_build_status.yml` to report fork build status on PRs. Update the comments accordingly. This builds on [#58579](#58579) (SPARK-59293), which fixes errors that abort the scheduled scan. ### Why are the changes needed? ASF is disabling the `pull_request_target` trigger in [November 2026][asf], ahead of GitHub's [December 2 deadline][github]. This is our only workflow using it. The scheduled updater already creates missing `Build` checks (SPARK-58660), so we don't need another workflow to replace the notifier. Fork builds still run on push, as usual; this PR only changes how their status gets reported. We should backport this PR to active maintenance branches that accept PRs, since `pull_request_target` uses the base branch's workflow. [asf]: https://infra.apache.org/github-actions-policy.html [github]: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target#default-policy-for-pull_request_target I intentionally kept this PR simple as I _really_ don't like that we have to maintain so much GitHub Actions-specific JavaScript just to manage our fork-based contribution workflow. In other words, in the interest of simplicity I intentionally left out: - Any mechanism to update PRs "live". - Testing infrastructure for these workflows and the JavaScript they contain. This infrastructure is not in the Spark project's area of expertise. It really feels like something we should be able to import and use from somewhere. Until then, I think we should strive to keep these workflows as simple as possible. ### Does this PR introduce _any_ user-facing change? No, nothing for end users. For contributors here on GitHub: - The "Notify test workflow" check goes away. - New `Build` checks appear as `queued` on the first scheduled pass and pick up the fork result on the next. The schedule is every 15 minutes, but GitHub can delay or drop runs. - If you push multiple updates to a PR between scans, only the latest commit gets a `Build` check. - The results link opens the fork's Actions run instead of its `Run / Check changes` check. ### How was this patch tested? Checked locally with actionlint, YAML parsing, and git diff --check. `update_build_status.yml` changes only comments, so the scheduled updater, which runs from master, already behaves as it will after this PR. In [run 40517915808](https://github.com/apache/spark/actions/runs/40517915808) it created Build checks for PRs that had none (#54953, #59831), and later scheduled runs synced them (#54953 is now success). Additionally, an agent replayed the unmodified updater against 150 open PRs with their Build checks hidden and writes intercepted: all 150 resolve to the same fork run, and the 36 action_required PRs would get identical checks. All 708 open master PRs currently have exactly one Build check, and 706 match their fork run. This PR can't fully test itself because `pull_request_target` runs the base branch's copy of the notifier. I will run additional tests post merge. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Copilot SDK in VS Code (version unavailable) Closes #59829 from nchammas/pr-target-actions-policy. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: Nicholas Chammas <[email protected]> (cherry picked from commit 6a02fc4) Signed-off-by: Nicholas Chammas <[email protected]>
|
Merge Summary:
Posted by |
…st_target` trigger ### What changes were proposed in this pull request? Remove `notify_test_workflow.yml` and rely on the existing `update_build_status.yml` to report fork build status on PRs. Update the comments accordingly. This builds on [#58579](#58579) (SPARK-59293), which fixes errors that abort the scheduled scan. ### Why are the changes needed? ASF is disabling the `pull_request_target` trigger in [November 2026][asf], ahead of GitHub's [December 2 deadline][github]. This is our only workflow using it. The scheduled updater already creates missing `Build` checks (SPARK-58660), so we don't need another workflow to replace the notifier. Fork builds still run on push, as usual; this PR only changes how their status gets reported. We should backport this PR to active maintenance branches that accept PRs, since `pull_request_target` uses the base branch's workflow. [asf]: https://infra.apache.org/github-actions-policy.html [github]: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target#default-policy-for-pull_request_target I intentionally kept this PR simple as I _really_ don't like that we have to maintain so much GitHub Actions-specific JavaScript just to manage our fork-based contribution workflow. In other words, in the interest of simplicity I intentionally left out: - Any mechanism to update PRs "live". - Testing infrastructure for these workflows and the JavaScript they contain. This infrastructure is not in the Spark project's area of expertise. It really feels like something we should be able to import and use from somewhere. Until then, I think we should strive to keep these workflows as simple as possible. ### Does this PR introduce _any_ user-facing change? No, nothing for end users. For contributors here on GitHub: - The "Notify test workflow" check goes away. - New `Build` checks appear as `queued` on the first scheduled pass and pick up the fork result on the next. The schedule is every 15 minutes, but GitHub can delay or drop runs. - If you push multiple updates to a PR between scans, only the latest commit gets a `Build` check. - The results link opens the fork's Actions run instead of its `Run / Check changes` check. ### How was this patch tested? Checked locally with actionlint, YAML parsing, and git diff --check. `update_build_status.yml` changes only comments, so the scheduled updater, which runs from master, already behaves as it will after this PR. In [run 40517915808](https://github.com/apache/spark/actions/runs/40517915808) it created Build checks for PRs that had none (#54953, #59831), and later scheduled runs synced them (#54953 is now success). Additionally, an agent replayed the unmodified updater against 150 open PRs with their Build checks hidden and writes intercepted: all 150 resolve to the same fork run, and the 36 action_required PRs would get identical checks. All 708 open master PRs currently have exactly one Build check, and 706 match their fork run. This PR can't fully test itself because `pull_request_target` runs the base branch's copy of the notifier. I will run additional tests post merge. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Copilot SDK in VS Code (version unavailable) Closes #59829 from nchammas/pr-target-actions-policy. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: Nicholas Chammas <[email protected]> (cherry picked from commit 6a02fc4) Signed-off-by: Nicholas Chammas <[email protected]>
…st_target` trigger ### What changes were proposed in this pull request? Remove `notify_test_workflow.yml` and rely on the existing `update_build_status.yml` to report fork build status on PRs. Update the comments accordingly. This builds on [#58579](#58579) (SPARK-59293), which fixes errors that abort the scheduled scan. ### Why are the changes needed? ASF is disabling the `pull_request_target` trigger in [November 2026][asf], ahead of GitHub's [December 2 deadline][github]. This is our only workflow using it. The scheduled updater already creates missing `Build` checks (SPARK-58660), so we don't need another workflow to replace the notifier. Fork builds still run on push, as usual; this PR only changes how their status gets reported. We should backport this PR to active maintenance branches that accept PRs, since `pull_request_target` uses the base branch's workflow. [asf]: https://infra.apache.org/github-actions-policy.html [github]: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target#default-policy-for-pull_request_target I intentionally kept this PR simple as I _really_ don't like that we have to maintain so much GitHub Actions-specific JavaScript just to manage our fork-based contribution workflow. In other words, in the interest of simplicity I intentionally left out: - Any mechanism to update PRs "live". - Testing infrastructure for these workflows and the JavaScript they contain. This infrastructure is not in the Spark project's area of expertise. It really feels like something we should be able to import and use from somewhere. Until then, I think we should strive to keep these workflows as simple as possible. ### Does this PR introduce _any_ user-facing change? No, nothing for end users. For contributors here on GitHub: - The "Notify test workflow" check goes away. - New `Build` checks appear as `queued` on the first scheduled pass and pick up the fork result on the next. The schedule is every 15 minutes, but GitHub can delay or drop runs. - If you push multiple updates to a PR between scans, only the latest commit gets a `Build` check. - The results link opens the fork's Actions run instead of its `Run / Check changes` check. ### How was this patch tested? Checked locally with actionlint, YAML parsing, and git diff --check. `update_build_status.yml` changes only comments, so the scheduled updater, which runs from master, already behaves as it will after this PR. In [run 40517915808](https://github.com/apache/spark/actions/runs/40517915808) it created Build checks for PRs that had none (#54953, #59831), and later scheduled runs synced them (#54953 is now success). Additionally, an agent replayed the unmodified updater against 150 open PRs with their Build checks hidden and writes intercepted: all 150 resolve to the same fork run, and the 36 action_required PRs would get identical checks. All 708 open master PRs currently have exactly one Build check, and 706 match their fork run. This PR can't fully test itself because `pull_request_target` runs the base branch's copy of the notifier. I will run additional tests post merge. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Copilot SDK in VS Code (version unavailable) Closes #59829 from nchammas/pr-target-actions-policy. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: Nicholas Chammas <[email protected]> (cherry picked from commit 6a02fc4) Signed-off-by: Nicholas Chammas <[email protected]>
…st_target` trigger ### What changes were proposed in this pull request? Remove `notify_test_workflow.yml` and rely on the existing `update_build_status.yml` to report fork build status on PRs. Update the comments accordingly. This builds on [#58579](#58579) (SPARK-59293), which fixes errors that abort the scheduled scan. ### Why are the changes needed? ASF is disabling the `pull_request_target` trigger in [November 2026][asf], ahead of GitHub's [December 2 deadline][github]. This is our only workflow using it. The scheduled updater already creates missing `Build` checks (SPARK-58660), so we don't need another workflow to replace the notifier. Fork builds still run on push, as usual; this PR only changes how their status gets reported. We should backport this PR to active maintenance branches that accept PRs, since `pull_request_target` uses the base branch's workflow. [asf]: https://infra.apache.org/github-actions-policy.html [github]: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target#default-policy-for-pull_request_target I intentionally kept this PR simple as I _really_ don't like that we have to maintain so much GitHub Actions-specific JavaScript just to manage our fork-based contribution workflow. In other words, in the interest of simplicity I intentionally left out: - Any mechanism to update PRs "live". - Testing infrastructure for these workflows and the JavaScript they contain. This infrastructure is not in the Spark project's area of expertise. It really feels like something we should be able to import and use from somewhere. Until then, I think we should strive to keep these workflows as simple as possible. ### Does this PR introduce _any_ user-facing change? No, nothing for end users. For contributors here on GitHub: - The "Notify test workflow" check goes away. - New `Build` checks appear as `queued` on the first scheduled pass and pick up the fork result on the next. The schedule is every 15 minutes, but GitHub can delay or drop runs. - If you push multiple updates to a PR between scans, only the latest commit gets a `Build` check. - The results link opens the fork's Actions run instead of its `Run / Check changes` check. ### How was this patch tested? Checked locally with actionlint, YAML parsing, and git diff --check. `update_build_status.yml` changes only comments, so the scheduled updater, which runs from master, already behaves as it will after this PR. In [run 40517915808](https://github.com/apache/spark/actions/runs/40517915808) it created Build checks for PRs that had none (#54953, #59831), and later scheduled runs synced them (#54953 is now success). Additionally, an agent replayed the unmodified updater against 150 open PRs with their Build checks hidden and writes intercepted: all 150 resolve to the same fork run, and the 36 action_required PRs would get identical checks. All 708 open master PRs currently have exactly one Build check, and 706 match their fork run. This PR can't fully test itself because `pull_request_target` runs the base branch's copy of the notifier. I will run additional tests post merge. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Copilot SDK in VS Code (version unavailable) Closes #59829 from nchammas/pr-target-actions-policy. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: Nicholas Chammas <[email protected]> (cherry picked from commit 6a02fc4) Signed-off-by: Nicholas Chammas <[email protected]>
### What changes were proposed in this pull request? Following on to #59829, refactor the scheduled build status updater (`update_build_status.yml`) completely: - Use batched GraphQL requests instead of per-PR REST requests. - Only update PR checks that need to be changed. Do a full sync periodically to catch build updates not associated with PR updates (e.g. a job is rerun with no new commit). - Clean up the log to only show key metrics and updates made. - Move the core logic into its own JavaScript file next to the workflow. Example of cleaned up log: ```text #60090 1dac863: queued -> in_progress #59954 48a5471: in_progress -> success 726 PRs checked, 36 skipped. Build checks: 7 updated, 2 created, 681 unchanged. ``` ### Why are the changes needed? Currently, an update pass takes about 10 minutes on a 15-minute schedule. That slows down status updates and makes changes to the workflow painful to test. Nearly all of that time goes to per-PR REST reads for PRs that haven't changed in weeks, and to rewriting checks that haven't changed. Fork runs usually finish within a few hours of a push. Of the latest fork runs on 690 open PRs, 8 were rerun more than 3 days after the push. The full sync picks those up. | | Before | After | |---|---|---| | Time per pass | ~10 min | ~45-60 s (~3-5 min for a full sync) | | PRs synced per pass | ~725 (all open) | ~170 | | API reads per pass | ~1,480 REST | 7 GraphQL | | Check updates per pass | ~685 | ~3 | | Log lines per pass | ~3,400 | ~10-15 | | Runner time per day | ~16 h | ~3 h | ### Does this PR introduce _any_ user-facing change? No, not for end-users. For contributors: `Build` checks update the same way, just sooner. A fork run rerun more than 3 days after the PR was last updated shows up on the next full sync, which is scheduled every 4 hours, or on the next pass after any PR activity (e.g. a comment). Contributor guidance now explains enabling Actions, using a non-master branch, and retriggering builds. To keep fork master as an upstream-sync branch, new Build checks for PRs from master provide `action_required` guidance rather than linking to a build, even if an older workflow ran there; existing run-linked checks continue updating. ### How was this patch tested? Via an agent: I replayed the old and new scripts read-only against live `apache/spark` PRs, intercepting writes. The new script made no write the old one didn't. All but one of the updates it skipped were no-ops. The remaining one was a stuck rerun on a PR outside the 3-day window, which the full sync covers. A full-sync replay synced all 727 open PRs. I also replayed both scripts with existing checks hidden, so every check had to be created. 724 of 725 creates matched, and each new check had its fork run's state. The one difference was a PR pushed a few minutes earlier with no fork run yet, which the new script left for the next pass. I ran quick and full syncs locally through `actions/github-script` against live PR data, intercepting writes and simulating responses, including ignored rerun updates. They checked 169 and 729 PRs in 57 seconds and 264 seconds, and logged 7 and 8 lines. All 26 focused tests passed, covering unchanged checks, applied and ignored updates, new checks, PRs from a fork's `master`, and errors. The test and dry-run harnesses load the JavaScript module directly. Replaying the 3 open PRs from a fork's `master` with their checks hidden, each got `action_required`. Tested on GitHub against my personal fork with a real `GITHUB_TOKEN`: a full read-only sync checked 728 PRs in 4m30s using 18 GraphQL points. A temporary fork PR verified check creation, updating the same check from queued to cancelled, and zero API writes on an unchanged repeat, with no duplicate checks. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: GitHub Copilot Closes #60098 from nchammas/build-status-optimization. Authored-by: Nicholas Chammas <[email protected]> Signed-off-by: Wenchen Fan <[email protected]> skip-checks: true
What changes were proposed in this pull request?
Remove
notify_test_workflow.ymland rely on the existingupdate_build_status.ymlto report fork build status on PRs. Update the comments accordingly.This builds on apache/spark#58579 (SPARK-59293), which fixes errors that abort the scheduled scan.
Why are the changes needed?
ASF is disabling the
pull_request_targettrigger in November 2026, ahead of GitHub's December 2 deadline. This is our only workflow using it.The scheduled updater already creates missing
Buildchecks (SPARK-58660), so we don't need another workflow to replace the notifier. Fork builds still run on push, as usual; this PR only changes how their status gets reported.We should backport this PR to active maintenance branches that accept PRs, since
pull_request_targetuses the base branch's workflow.I intentionally kept this PR simple as I really don't like that we have to maintain so much GitHub Actions-specific JavaScript just to manage our fork-based contribution workflow.
In other words, in the interest of simplicity I intentionally left out:
This infrastructure is not in the Spark project's area of expertise. It really feels like something we should be able to import and use from somewhere. Until then, I think we should strive to keep these workflows as simple as possible.
Does this PR introduce any user-facing change?
No, nothing for end users.
For contributors here on GitHub:
Buildchecks appear asqueuedon the first scheduled pass and pick up the fork result on the next. The schedule is every 15 minutes, but GitHub can delay or drop runs.Buildcheck.Run / Check changescheck.How was this patch tested?
Checked locally with actionlint, YAML parsing, and git diff --check.
update_build_status.ymlchanges only comments, so the scheduled updater, which runs from master, already behaves as it will after this PR. In run 40517915808 it created Build checks for PRs that had none (#54953, #59831), and later scheduled runs synced them (#54953 is now success).Additionally, an agent replayed the unmodified updater against 150 open PRs with their Build checks hidden and writes intercepted: all 150 resolve to the same fork run, and the 36 action_required PRs would get identical checks. All 708 open master PRs currently have exactly one Build check, and 706 match their fork run.
This PR can't fully test itself because
pull_request_targetruns the base branch's copy of the notifier. I will run additional tests post merge.Was this patch authored or co-authored using generative AI tooling?
Generated-by: Copilot SDK in VS Code (version unavailable)