Skip to content

[SPARK-60578][INFRA] Migrate build status workflow off of pull_request_target trigger - #59829

Closed
nchammas wants to merge 3 commits into
apache:masterfrom
nchammas:pr-target-actions-policy
Closed

nchammas wants to merge 3 commits into
apache:masterfrom
nchammas:pr-target-actions-policy

Conversation

@nchammas

@nchammas nchammas commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

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 apache/spark#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, ahead of GitHub's December 2 deadline. 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.

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 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)

@nchammas nchammas added the ai-review-requested Trigger AI review by Omnigent on Databricks label Oct 27, 2026
spark-test

This comment was marked as resolved.

@spark-test spark-test removed the ai-review-requested Trigger AI review by Omnigent on Databricks label Oct 27, 2026
@cloud-fan

Copy link
Copy Markdown
Contributor

@nchammas can you rebase? #58579 is merged now

@nchammas
nchammas requested a review from cloud-fan October 27, 2026 15:30
@HyukjinKwon HyukjinKwon added the ai-review-requested Trigger AI review by Omnigent on Databricks label Oct 27, 2026

@HyukjinKwon HyukjinKwon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if it works

@spark-test

This comment was marked as resolved.

@spark-test spark-test removed the ai-review-requested Trigger AI review by Omnigent on Databricks label Oct 27, 2026
@HyukjinKwon HyukjinKwon added the ai-review-requested Trigger AI review by Omnigent on Databricks label Oct 27, 2026
spark-test

This comment was marked as resolved.

@spark-test spark-test removed the ai-review-requested Trigger AI review by Omnigent on Databricks label Oct 27, 2026

@cloud-fan cloud-fan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 queued check, 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 queued instead of in_progress for 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.

@nchammas

nchammas commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Post-merge TODOs:

  • Monitor the build workflow for at least an hour after merge to confirm it is updating PR check statuses as expected.
  • Backport this change to all active branches that are accepting PRs. I think that means going all the way back to branch-3.5?
  • In a future PR address the feedback about the Build check's initial status; make it more precise than queued when possible.

I'm going to merge this in and monitor the outcome for the next few hours.

@nchammas nchammas closed this in 6a02fc4 Nov 3, 2026
@nchammas

nchammas commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Merge Summary:

Posted by merge_spark_pr.py

@nchammas
nchammas deleted the pr-target-actions-policy branch November 3, 2026 22:06
@nchammas

nchammas commented Nov 3, 2026

Copy link
Copy Markdown
Contributor Author

Post merge observations so far:

  • The scheduled update is working. I see the new build status on [SPARK-60615][INFRA] Restore PR labeler (without pull_request_target) #59862, for example.
  • The build check links directly to the workflow summary page, which is nice (and expected).
  • The "queued" status started off stale due to the delay between updates, as noted in the review comments above. I will address this in a new PR.
  • The update build status job takes a long time relative to what it does. 10 minutes seems like way too much. It is almost certainly wasting time doing things in a frantic loop vs. batching operations. I will look into this as well.

I will wait until tomorrow before backporting this to all branches going back to branch-3.5.

nchammas added a commit that referenced this pull request Nov 6, 2026
…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]>
nchammas added a commit that referenced this pull request Nov 6, 2026
…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]>
@nchammas

nchammas commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Merge Summary:

Posted by merge_spark_pr.py

nchammas added a commit that referenced this pull request Nov 6, 2026
…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]>
@nchammas

nchammas commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Merge Summary:

Posted by merge_spark_pr.py

nchammas added a commit that referenced this pull request Nov 6, 2026
…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]>
nchammas added a commit that referenced this pull request Nov 6, 2026
…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]>
nchammas added a commit that referenced this pull request Nov 6, 2026
…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]>
@nchammas

nchammas commented Nov 6, 2026

Copy link
Copy Markdown
Contributor Author

Merge Summary:

Posted by merge_spark_pr.py

cloud-fan pushed a commit that referenced this pull request Nov 10, 2026
### 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants