This is an automated email from the ASF dual-hosted git repository.
cloud-fan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new d8834302f0b0 [SPARK-59293][INFRA] Fix update_build_status.yml to map
non-standard conclusions
d8834302f0b0 is described below
commit d8834302f0b0c5696dd246e194e9f3f2d76457a9
Author: Tornike Gurgenidze <[email protected]>
AuthorDate: Tue Oct 27 13:57:00 2026 +0000
[SPARK-59293][INFRA] Fix update_build_status.yml to map non-standard
conclusions
### 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
Closes #58579 from tokoko/fix-build-status-invalid-conclusion.
Lead-authored-by: Tornike Gurgenidze <[email protected]>
Co-authored-by: Wenchen Fan <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
---
.github/workflows/update_build_status.yml | 60 ++++++++++++++++++++++++++-----
1 file changed, 52 insertions(+), 8 deletions(-)
diff --git a/.github/workflows/update_build_status.yml
b/.github/workflows/update_build_status.yml
index 1f1e8fafd5d9..3ecbe188f10d 100644
--- a/.github/workflows/update_build_status.yml
+++ b/.github/workflows/update_build_status.yml
@@ -112,6 +112,28 @@ jobs:
const isActionRequiredBuildCheck = (cr) =>
cr.name == 'Build' && cr.conclusion == 'action_required';
+ // PRs whose sync failed on this pass: listing their check runs,
PATCHing a Build
+ // check, or backfilling one. A failed call skips only that PR (it
is retried on the
+ // next pass) instead of aborting the whole scheduled run and
leaving every PR later
+ // in the iteration unsynced. The step still fails after the pass,
so an error that
+ // keeps recurring (e.g. a 422 for a value the endpoint rejects)
stays visible instead
+ // of hiding in the log of a green run. Transient fork-run lookup
failures (see
+ // isTransientError) are not recorded: they also skip the PR until
a later pass, but
+ // leave the step green.
+ const failedPRs = new Set();
+
+ // PATCH a Build check-run, recording a failure against the PR
instead of throwing.
+ const updateCheckRun = async (pr, body) => {
+ try {
+ await github.request('PATCH
/repos/{owner}/{repo}/check-runs/{check_run_id}', body);
+ } catch (error) {
+ console.error(' Updating check run ' + body.check_run_id +
' failed; '
+ + 'will retry next pass');
+ console.error(error);
+ failedPRs.add(pr.number);
+ }
+ };
+
// Iterate open PRs
for await (const prs of github.paginate.iterator(endpoint,params))
{
// Each page
@@ -119,7 +141,15 @@ jobs:
console.log('SHA: ' + pr.head.sha)
console.log(' Mergeable status: ' + pr.mergeable_state)
if (pr.mergeable_state == null ||
maybeReady.includes(pr.mergeable_state)) {
- const checkRuns = await listCheckRuns(pr.head.sha)
+ let checkRuns
+ try {
+ checkRuns = await listCheckRuns(pr.head.sha)
+ } catch (error) {
+ console.error(' Listing check runs failed for this PR;
will retry next pass')
+ console.error(error)
+ failedPRs.add(pr.number)
+ continue
+ }
// Does this SHA already carry an action_required Build
check? notify (or an
// earlier pass of this updater) writes one when no fork run
was found. It is
@@ -179,14 +209,22 @@ jobs:
// Keep syncing the status of the checks
if (run.data.status == 'completed') {
- console.log(' Run ' + cr.id + ': set status (' +
run.data.status + ') and conclusion (' + run.data.conclusion + ')')
- const response = await github.request('PATCH
/repos/{owner}/{repo}/check-runs/{check_run_id}', {
+ // PATCH /check-runs accepts only success | failure |
neutral |
+ // cancelled | timed_out | action_required | skipped,
but a workflow_run
+ // may also conclude startup_failure / stale, or
report a null conclusion.
+ // Those all mean the run produced no test result, so
they become failure.
+ const conclusions = ['success', 'failure', 'neutral',
'cancelled',
+ 'timed_out', 'action_required', 'skipped']
+ const conclusion =
conclusions.includes(run.data.conclusion)
+ ? run.data.conclusion : 'failure'
+ console.log(' Run ' + cr.id + ': set status (' +
run.data.status + ') and conclusion (' + run.data.conclusion + ' -> ' +
conclusion + ')')
+ await updateCheckRun(pr, {
owner: context.repo.owner,
repo: context.repo.repo,
check_run_id: cr.id,
output: cr.output,
status: run.data.status,
- conclusion: run.data.conclusion,
+ conclusion: conclusion,
details_url: run.data.details_url
})
} else {
@@ -194,7 +232,7 @@ jobs:
// but workflow_run may also report requested /
waiting / pending.
const status = run.data.status == 'in_progress' ?
'in_progress' : 'queued'
console.log(' Run ' + cr.id + ': set status (' +
run.data.status + ' -> ' + status + ')')
- const response = await github.request('PATCH
/repos/{owner}/{repo}/check-runs/{check_run_id}', {
+ await updateCheckRun(pr, {
owner: context.repo.owner,
repo: context.repo.repo,
check_run_id: cr.id,
@@ -301,9 +339,9 @@ jobs:
// even though its output still parses as syncable, so it
too is excluded.
//
// The recheck listing and the create below are wrapped so
a failure on one
- // PR degrades to "skip this PR, retry next pass" rather
than aborting the
- // whole scheduled run and starving every PR later in the
iteration - the same
- // reason the sync loop above guards its calls.
+ // PR degrades to "skip this PR, retry next pass" (see
failedPRs) rather than
+ // aborting the whole scheduled run and starving every PR
later in the
+ // iteration - the same reason the sync loop above guards
its calls.
try {
const recheck = await listCheckRuns(pr.head.sha)
const existingBuild = recheck.some(cr =>
@@ -379,9 +417,15 @@ jobs:
} catch (error) {
console.error(' Backfill failed for this PR; will retry
next pass')
console.error(error)
+ failedPRs.add(pr.number)
continue
}
}
}
}
}
+
+ if (failedPRs.size > 0) {
+ core.setFailed('Failed to sync ' + failedPRs.size + ' PR(s),
will retry next pass: #'
+ + [...failedPRs].join(', #'))
+ }
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
