[SPARK-59293][INFRA] Fix update_build_status.yml to map non-standard conclusions by tokoko · Pull Request #58579 · apache/spark

tokoko · 2026-09-07T17:50:26Z

What changes were proposed in this pull request?

This PR fixes update_build_status.yml so that it coerces non-standard workflow run conclusions to a value accepted by the PATCH /repos/{owner}/{repo}/check-runs/{check_run_id} endpoint. run.data.conclusion is mapped to itself when it is one of the conclusions the endpoint accepts, and to failure otherwise, instead of passing the raw value through. This mirrors the mapping of non-standard run statuses in the else branch a few lines below, added by SPARK-57124.

It also isolates per-PR sync failures, so that a single uncaught error can no longer end the whole scheduled pass:

The per-PR check-run listing is wrapped in a try/catch; a failure skips only that PR, which is retried on the next pass.
Both check-run PATCH calls go through an updateCheckRun helper that logs and records a failure instead of throwing.
Backfill failures, which already skipped the PR, are now recorded too.
After the full pass, the step calls core.setFailed naming every recorded PR, so a recurring error like this 422 still turns the scheduled run red instead of hiding in the log of a green run. Transient fork-run lookup failures (5xx, rate limiting) still skip the PR without failing the step, as before.

Successful syncs send the same PATCH bodies as before.

Why are the changes needed?

The scheduled job has been failing on every run for quite a while with startup_failure is not a member of ["success", "failure", "neutral", "cancelled", "timed_out", "action_required", "skipped"].

Because the error was uncaught, it ended the pass and left every open PR later in the iteration unsynced, so their Build checks could stay queued after their fork runs had completed. Isolating failures per PR keeps one rejected value from stalling the whole job again.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Extracted the script from the YAML and ran it in node against a mocked GitHub API whose PATCH rejects invalid values with a 422, like the real endpoint. Injected a 502 on one PR's check-run listing, a 422 on another's PATCH, and a 500 on a third's backfill: the script synced every other PR, including the one after all three failures, then failed the step naming just those three. With no injected failures it sent the same PATCHes as before and did not fail the step. Not run against live data.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Opus 5

…workflow run conclusions

uros-b

This looks minimal and complete / correct, so thank you @tokoko! However, let's also make sure to ping @dongjoon-hyun who worked on related SPARK-57124

szehon-ho · 2026-10-26T19:04:04Z

This is still affecting the status updater as of October 26. The two latest completed runs both abort with HTTP 422 when publishing startup_failure as a check-run conclusion:

The uncaught error stops processing subsequent PRs. For example, #59036's upstream Build check remains queued, although its fork CI run has completed successfully after rerunning the UI test.

I reviewed the conclusion mapping here, and it looks correct for this failure.

szehon-ho

if this is the case, i think let's merge to let the CI status report successfully on prs. any thoughts @dongjoon-hyun @cloud-fan @HyukjinKwon ?

A failed check-run listing or PATCH on one PR no longer aborts the whole scheduled pass and leaves every later PR unsynced. Each failure, including a failed backfill, now skips only that PR (retried on the next pass), and the step fails after the pass so a recurring error, such as a 422 on a value the endpoint rejects, stays visible instead of hiding in the log of a green run. Co-authored-by: Isaac <[email protected]>

cloud-fan · 2026-10-26T20:58:52Z

I pushed 4f454a7 on top of this PR. The conclusion mapping fixes the current failure, but the loop over open PRs still lets any single uncaught error end the whole scheduled pass, which leaves every later PR unsynced. That is how one rejected value stalled the whole job here.

The new commit:

Wraps the per-PR check-run listing in a try/catch, so a failure skips only that PR and it is retried on the next pass.
Routes both check-run PATCH calls through an updateCheckRun helper that logs and records a failure instead of throwing.
Records backfill failures too. The backfill already skipped the PR on error; it just didn't report it.
Calls core.setFailed after the full pass if any PR was recorded, naming those PRs. A recurring error like this 422 still turns the scheduled run red instead of hiding in the log of a green run.

Successful syncs send the same PATCH bodies as before.

Testing: I extracted the script from the YAML and ran it in node against a mocked GitHub API whose PATCH rejects invalid values with a 422, like the real endpoint. I injected a 502 on one PR's check-run listing, a 422 on another's PATCH, and a 500 on a third's backfill. The script synced every other PR, including the one after all three failures, then failed the step naming just those three. With no injected failures it sent the same PATCHes as the previous head and did not fail the step. I did not run it against live data.

Feel free to adjust or drop the commit if you'd rather keep this PR to the mapping alone.

The failedPRs comment implied every recurring per-PR error turns the scheduled run red, but transient fork-run lookup failures still skip the PR without recording it. Name the three recorded failure sites and note that transient fork-run lookups leave the step green. Co-authored-by: Isaac <[email protected]>

cloud-fan · 2026-10-27T13:56:12Z

the failed tests are unrelated, thanks, merging to master!

cloud-fan · 2026-10-27T14:01:32Z

Merge Summary:

merged into master d883430

Posted by merge_spark_pr.py

…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]>

…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]>

[SPARK-59293][INFRA] Fix update_build_status.yml to map non-standard …

9f5b3ae

…workflow run conclusions

tokoko mentioned this pull request Sep 8, 2026

[SPARK-49933][SQL] Push WHEN NOT MATCHED BY SOURCE predicates to target #52185

Open

uros-b reviewed Sep 8, 2026

View reviewed changes

uros-b requested a review from dongjoon-hyun September 8, 2026 10:18

szehon-ho approved these changes Oct 26, 2026

View reviewed changes

szehon-ho mentioned this pull request Oct 26, 2026

[SPARK-59778][SQL] Add display-only properties to the Table API #59036

Closed

cloud-fan approved these changes Oct 26, 2026

View reviewed changes

qingfureal mentioned this pull request Oct 26, 2026

[SPARK-59598][SQL] GetMapValue and ElementAt must not match a null map key #58875

Open

nchammas mentioned this pull request Oct 27, 2026

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

Closed

cloud-fan closed this in d883430 Oct 27, 2026

Navigation Menu

Repository navigation

[SPARK-59293][INFRA] Fix update_build_status.yml to map non-standard conclusions - #58579

[SPARK-59293][INFRA] Fix update_build_status.yml to map non-standard conclusions#58579
tokoko wants to merge 3 commits into
apache:masterfrom
tokoko:fix-build-status-invalid-conclusion

tokoko commented Sep 7, 2026 •

edited by cloud-fan

Loading

Uh oh!

uros-b left a comment

Uh oh!

szehon-ho commented Oct 26, 2026

Uh oh!

szehon-ho left a comment

Uh oh!

cloud-fan commented Oct 26, 2026

Uh oh!

cloud-fan commented Oct 27, 2026 •

edited

Loading

Uh oh!

cloud-fan commented Oct 27, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

tokoko commented Sep 7, 2026 • edited by cloud-fan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Was this patch authored or co-authored using generative AI tooling?

Uh oh!

uros-b left a comment

Choose a reason for hiding this comment

Uh oh!

szehon-ho commented Oct 26, 2026

Uh oh!

szehon-ho left a comment

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Oct 26, 2026

Uh oh!

cloud-fan commented Oct 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cloud-fan commented Oct 27, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tokoko commented Sep 7, 2026 •

edited by cloud-fan

Loading

cloud-fan commented Oct 27, 2026 •

edited

Loading