This is an automated email from the ASF dual-hosted git repository.
andygrove pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/datafusion-comet.git
The following commit(s) were added to refs/heads/main by this push:
new f29a236128 ci: add a Required Checks aggregator job so main can have a
required status check (#5842)
f29a236128 is described below
commit f29a236128b3c0b175bb77b5b1f5192356c3056a
Author: Andy Grove <[email protected]>
AuthorDate: Fri Sep 11 08:17:02 2026 -0600
ci: add a Required Checks aggregator job so main can have a required status
check (#5842)
* ci: move the job routing policy out of ci.yml expressions and into
compute-changes.py
Every heavy job in `ci.yml` carried a four-line `${{ }}` expression
combining
a path-filter output, an event-name test, an opt-in label test, and a
special
case for `labeled` events. Ten jobs, ten near-identical copies, none of them
testable outside a real workflow run.
Fold that policy into `dev/ci/compute-changes.py` next to the path filters
it
was already being ANDed with. Each job's gate becomes:
if: needs.changes.outputs.spark_3_5 == 'true'
and the routing lives in one readable table:
POLICY = {
"spark_3_5": ["pr", "push"],
"spark_3_4": ["push", "label:run-spark-3.4-tests"],
...
}
`ci.yml` loses 62 lines net. No behaviour changes: the `changes` job now
receives the event name, action, added label and PR labels, and applies the
same rules the expressions did.
The point of moving it is that it can now be tested. `check-ci-config.py`
grows `POLICY_CASES`, which pins the expected job set for each event shape
--
including that a non-gating label such as dependabot's `dependencies` starts
nothing, which is issue #5007 and was previously only enforceable by reading
YAML carefully.
Writing `"pr"` alongside a `"label:"` tier reads as "runs on every PR, and
also
when labelled" but the label check wins and the "pr" is dead, so
`check-ci-config.py` now rejects that combination rather than letting it be
a
silent no-op.
Verified equivalent with a differential harness: the pre-refactor `if:`
expressions were extracted from ci.yml at the merge base, translated to
Python,
and evaluated against the new POLICY over 1464 (job, event) combinations --
every event name, pull_request action, label set and added-label pairing.
Zero
mismatches, and three seeded regressions (renaming a gating label, dropping
a
label gate, dropping the labeled-event narrowing) are each caught.
Groundwork for #5838: adding the merge queue then means adding one tier to
POLICY rather than editing ten YAML expressions.
* ci: add a Required Checks aggregator job so main can have a required
status check
`main` currently requires no status checks, only one approving review.
Adding
one is blocked by how the umbrella workflow reports: every heavy job in
`ci.yml` is a thin caller of a reusable workflow, and the check name a
caller
publishes depends on whether it ran.
skipped by `if:` one check run named exactly `PR Build (Linux)`
actually ran only `PR Build (Linux) / Spark 4.1, JDK 17 [exec]` and
friends, and no bare `PR Build (Linux)` at all
Confirmed against bdd2aeb (md-only, everything skipped) and 36caf8e (heavy
jobs ran). No name is reported in both cases, so requiring the bare name
would
block every code change and requiring a nested name would block every
docs-only
change. Both hang waiting for a check that never arrives rather than
failing,
and a required context that never reports also blocks the merge that would
fix
`.asf.yaml` -- only INFRA can remove a required check by hand at that point.
Add `required_checks`, a flat job that `needs:` every other job and reports
on
every event. It runs `if: always()` and treats `skipped` as a pass, so it
goes
red only when an upstream job reports `failure` or `cancelled`.
Nothing requires it yet. This lands on its own so the check can be observed
on
real pull requests before `.asf.yaml` names it, which is the step that is
expensive to get wrong.
`dev/ci/check-ci-config.py` gains two invariants: every `ci.yml` job except
`docs` must appear in `required_checks.needs`, and once `.asf.yaml` does
declare a required context, the job's `name:` must match it.
Part of #5838.
* ci: give the Required Checks job an empty permissions block
The aggregator reads only the `needs` context, so it has no use for the
GITHUB_TOKEN. Declaring `permissions: {}` on the job drops the token
entirely and clears the CodeQL "Workflow does not contain permissions"
finding, without touching the token scope any other job in ci.yml gets.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B
* ci: tighten the required-checks invariants in check-ci-config.py
Widen the job-id regexes to accept `-` and uppercase, which GitHub allows.
A `spark-4-2:` job missing from `required_checks.needs` passed the coverage
check silently before this change.
Scope the `.asf.yaml` parse to the `main:` entry under `protected_branches`.
It previously collected every `contexts:` list in the file, so a release
branch requiring its own context produced a false failure that blamed main.
Drop the em dashes from the new README prose.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B
* ci: retry or tolerate infra-only step failures on the required path
Once Required Checks is a required context, any red job evicts the PR from
the merge queue. The first two runs on this branch each failed in a step
that runs after the test verdict is known or before any test has started,
and in neither case was a test involved.
- java-test: mark the test-report upload continue-on-error. It runs on green
jobs, nothing downstream consumes the reports, and a FinalizeArtifact 403
must not turn a passing run red. The failure-only uploads are unchanged.
- iceberg_spark_test_reusable: route the shard-inventory upload through
upload-artifact-retry. iceberg-spark-shard-coverage consumes it, so it
already fell under the README rule for artifacts a later job downloads.
- pr_build_linux: split Lint Scala (syntactic) into a retried no-op
`cs launch scalafix -- --version` that populates the cache, then the real
check under `--mode offline`, so a nonzero exit there is only ever a lint
violation.
- ci.yml preflight: retry the actionlint download and fetch the installer
to a file instead of piping it into bash.
Closes #5860
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B
* ci: publish label-run verdicts under a name nothing requires
ci.yml also fires on `labeled`, and on that event POLICY skips the PR tier
because it already ran at the same commit. The label run's aggregate says
nothing about the commit's applicable suites, but GitHub keeps only the most
recent check run per name per commit, so publishing it as `Required Checks`
would let a `dependencies` label that lands a minute after a push mark the
commit green while the real run is still going (the mechanism from #5007).
Make the aggregator's `name:` an expression that routes label runs to
`Required Checks (label run)`. Skipping the job on `labeled` would not help:
a skipped check run still carries the name and still counts as passing.
check-ci-config.py now requires that expression shape, rejects a `labeled`
branch equal to the required name, and rejects an `.asf.yaml` that requires
the label-run name. The commit-run name is what has to match `.asf.yaml`.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_01BtAqq4YJsk8uk42c7vHk8B
---------
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.github/actions/java-test/action.yaml | 6 +-
.github/workflows/README.md | 85 +++++++--
.github/workflows/ci.yml | 87 ++++++++-
.github/workflows/iceberg_spark_test_reusable.yml | 4 +-
.github/workflows/pr_build_linux.yml | 21 ++-
dev/ci/check-ci-config.py | 206 +++++++++++++++++++++-
6 files changed, 384 insertions(+), 25 deletions(-)
diff --git a/.github/actions/java-test/action.yaml
b/.github/actions/java-test/action.yaml
index ef640da3e2..42078b3c44 100644
--- a/.github/actions/java-test/action.yaml
+++ b/.github/actions/java-test/action.yaml
@@ -138,7 +138,10 @@ runs:
if: failure()
# These three stay on the plain action rather than
# ../upload-artifact-retry: a local action calling another local action
- # is untested in this repo, and these only run on already-failing jobs.
+ # is untested in this repo. The two failure-only uploads run on jobs that
+ # are already red. The test-report upload also runs on green jobs, so it
+ # is continue-on-error: a FinalizeArtifact 403 must not turn a passing
test
+ # run into a red check, and nothing downstream consumes the reports.
uses: actions/upload-artifact@v7
with:
name: crash-logs-${{ inputs.artifact_name }}
@@ -165,6 +168,7 @@ runs:
if-no-files-found: ignore
- name: Upload test results
if: ${{ !cancelled() && inputs.upload-test-reports == 'true' }}
+ continue-on-error: true
uses: actions/upload-artifact@v7
with:
name: java-test-reports-${{ inputs.artifact_name }}
diff --git a/.github/workflows/README.md b/.github/workflows/README.md
index 935e11272d..f98918b24a 100644
--- a/.github/workflows/README.md
+++ b/.github/workflows/README.md
@@ -42,6 +42,15 @@ is a `workflow_call` reusable invoked from the umbrella.
spark_4_1 iceberg_1_10
run-iceberg-tests
iceberg_1_11
+ | |
|
+
+-----------------------------------+-----------------------------------+
+ v
+ +-----------------------+
+ | required_checks | ubuntu-slim
+ | one flat name that |
+ | is safe to require |
+ +-----------------------+
+
reusable workflows invoked via `uses:`:
pr_build_linux.yml spark_sql_test_reusable.yml
pr_build_macos.yml iceberg_spark_test_reusable.yml
@@ -67,6 +76,7 @@ is a `workflow_call` reusable invoked from the umbrella.
| `iceberg_1_8` | push, **or** PR with `run-iceberg-tests` label |
Iceberg sources |
| `iceberg_1_9` | push, **or** PR with `run-iceberg-tests` label |
Iceberg sources |
| `iceberg_1_10` | push, **or** PR with `run-iceberg-tests` label |
Iceberg sources |
+| `required_checks` | always, after every job above except `docs` |
none (always runs) |
A heavy job appears in the PR's checks list as a `skipped` entry whenever
its path filter or event criteria don't match. Skipped checks count as
@@ -93,6 +103,14 @@ Two rules keep those runs from corrupting the PR's status:
- On a `labeled` event, `POLICY` reports false for every job the new label does
not gate. Without that, applying a single label re-ran the entire heavy
pipeline at a commit that had already been tested.
+- On a `labeled` event, `required_checks` publishes its verdict as
+ `Required Checks (label run)`, not `Required Checks`. Because the PR tier is
+ skipped on that event, the label run's aggregate says nothing about the
+ commit's applicable suites, and GitHub keeps only the most recent check run
+ per name per commit. Under the real name, a `dependencies` label landing a
+ minute after a push would mark the commit green while the commit run was
+ still going. Skipping the job would not help either, since a skipped check
+ run still carries the name and still counts as passing.
`run-spark-4.1-tests` gates nothing: `spark_4_1` already runs on every PR.
@@ -198,10 +216,26 @@ though the content already uploaded. Its client only
retries
built-in step retry. Use `./.github/actions/upload-artifact-retry` instead for
any artifact a later job consumes: same inputs and outputs, three attempts,
15s then 45s backoff. Attempts 2 and 3 force `overwrite: true`, so the name
-must belong to exactly one producer in the run (see above). The diagnostic
-uploads inside `./.github/actions/java-test` stay on the plain action, since a
-local action calling another local action is untested here and those run only
-on already-failing jobs.
+must belong to exactly one producer in the run (see above). The uploads inside
+`./.github/actions/java-test` stay on the plain action, since a local action
+calling another local action is untested here. Its two failure-only uploads run
+on jobs that are already red. Its test-report upload also runs on green jobs
+and is `continue-on-error: true`: nothing downstream consumes the reports, and
+a `FinalizeArtifact` 403 must not turn a passing test run into a red check.
+
+**Tool downloads.** `Lint Scala (syntactic)` splits the coursier download from
+the lint: a `Fetch scalafix` step retries a no-op `cs launch ... -- --version`
+three times, and the check itself then runs `cs launch --mode offline` against
+the populated cache, so a nonzero exit there can only be a lint violation.
+`preflight` retries the actionlint download the same way, and fetches the
+installer to a file rather than piping it into `bash` so a truncated download
+cannot run a partial script.
+
+Every one of these steps runs after the test verdict is already known, or
+before any test has started. Once `Required Checks` is a required context
+(see below), a red job from any of them evicts the PR from the merge queue,
+which is why plain network flakes are worth retrying rather than re-running
+the whole pipeline by hand.
**Maven wrapper bootstrap.** `./.github/actions/java-test` retries
`./mvnw --version` with exponential backoff, so a failed download of the Maven
@@ -209,17 +243,32 @@ distribution does not surface as a test failure.
## Branch protection
-Required-check names changed when these workflows were consolidated. The
-umbrella exposes per-job names like `CI / pr_build_linux / Lint`,
-`CI / spark_3_5 / linux-test (...)`, etc. Update repository branch
-protection rules to point at the new names; the old standalone workflow
-names (`Spark SQL Tests (Spark 3.5)`, `PR Build (Linux)`, ...) no longer
-exist as top-level workflows.
-
-`.asf.yaml` currently declares no `required_status_checks` for `main`, only
-`required_approving_review_count: 1`. Anything added there must be a name that
-never legitimately reports `skipped`, because GitHub counts a skipped check as
-passing. The bare caller-job names (`PR Build (Linux)`, `Spark SQL Tests
-(Spark 3.5)`, ...) are not such names: a reusable workflow that actually runs
-publishes only its child jobs, so the bare name shows up solely when the caller
-was skipped.
+`.asf.yaml` declares no `required_status_checks` for `main` today, only
+`required_approving_review_count: 1`.
+
+Adding one is not as simple as naming a job, because a caller of a reusable
+workflow publishes a _different check name_ depending on whether it ran:
+
+| Caller state | Check runs published |
+| ---------------- | -------------------------------------------------------- |
+| skipped by `if:` | one run named exactly `PR Build (Linux)`, `skipped` |
+| ran | only `PR Build (Linux) / Spark 4.1, JDK 17 [exec]`, etc. |
+
+No name is reported in both cases. Requiring the bare name would block every
+code change; requiring a nested name would block every docs-only change. Both
+hang waiting for a check that never arrives rather than failing, and a required
+context that never reports also blocks the merge that would fix `.asf.yaml`.
+Recovering from that needs an INFRA Jira ticket.
+
+The `required_checks` job at the bottom of `ci.yml` exists to be the one name
+that is safe to require. It is flat, so it reports on every event; it runs
+`if: always()` and treats `skipped` as a pass, so it only goes red when an
+upstream job reports `failure` or `cancelled`.
+
+`dev/ci/check-ci-config.py` enforces that every `ci.yml` job except `docs`
+appears in `required_checks.needs`, and that the job's `name:` is the
+expression that routes `labeled` runs to a separate name (see "Label events"
+above). Once `.asf.yaml` does declare a required context for `main`, it also
+enforces that the commit-run name still matches it and that the label-run name
+is never the one required. All of that is silent when broken and expensive to
+recover from.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index f53d854ba6..1bb96a43ab 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -99,9 +99,24 @@ jobs:
run: python3 dev/ci/check-ci-config.py
- name: Install actionlint
+ # Pure network, and preflight gates every other job, so a single reset
+ # connection here would fail the whole run. Download to a file rather
+ # than pipe into bash so a failed download cannot run a partial script.
run: |
- curl -sSfL
https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash
| bash
- echo "$PWD" >> $GITHUB_PATH
+ for attempt in 1 2 3; do
+ if curl -sSfL --retry 3 --retry-all-errors -o
download-actionlint.bash \
+
https://raw.githubusercontent.com/rhysd/actionlint/main/scripts/download-actionlint.bash
\
+ && bash download-actionlint.bash; then
+ break
+ fi
+ if [ "$attempt" = 3 ]; then
+ echo "::error::actionlint download failed after 3 attempts."
+ exit 1
+ fi
+ echo "::warning::actionlint download failed (attempt $attempt of
3); retrying in $((attempt * 10))s."
+ sleep $((attempt * 10))
+ done
+ echo "$PWD" >> "$GITHUB_PATH"
- name: Lint GitHub Actions workflows
run: actionlint -color --shellcheck=off
@@ -305,3 +320,71 @@ jobs:
spark-short: '4.1'
spark-full: '4.1.3'
java: 17
+
+ # ---------------------------------------------------------------------------
+ # required_checks: one flat job that aggregates every other job's result, so
+ # that `main` has a single name it can safely require.
+ #
+ # Nothing requires it yet. It is landing first, on its own, so that we can
+ # watch it report on real pull requests before `.asf.yaml` names it in
+ # `required_status_checks` for `main`.
+ #
+ # None of the jobs above can be required directly, because the name a caller
+ # of a reusable workflow publishes depends on whether it ran:
+ #
+ # skipped by `if:` one check run named exactly `PR Build (Linux)`
+ # actually ran only `PR Build (Linux) / Spark 4.1, JDK 17 [exec]`,
+ # ... and no bare `PR Build (Linux)` at all
+ #
+ # So requiring the bare name would block every code change, and requiring a
+ # nested name would block every docs-only change. Both hang rather than fail,
+ # and a required context that never reports also locks `.asf.yaml` itself,
+ # which then needs an INFRA ticket to unwedge. Aggregating into one flat job,
+ # whose name is published on every event, avoids the whole class of problem.
+ #
+ # `if: always()` is what makes this work: without it the job inherits the
+ # default `success()` and is itself skipped the moment any dependency fails.
+ #
+ # The name is an expression because `ci.yml` also fires on `labeled`, and on
+ # that event POLICY deliberately skips the PR tier (it already ran at this
+ # commit). A label run's verdict therefore says nothing about the commit's
+ # applicable suites, yet GitHub keeps only the most recent check run per name
+ # per commit, so publishing it as `Required Checks` would let a
`dependencies`
+ # label turn a still-running or red commit run green (issue #5007). Label
+ # runs publish under a name nothing requires instead. Skipping the job here
+ # would not help: a skipped check run still carries the name and still counts
+ # as passing. dev/ci/check-ci-config.py enforces both halves of this.
+ # ---------------------------------------------------------------------------
+ required_checks:
+ name: ${{ github.event.action == 'labeled' && 'Required Checks (label
run)' || 'Required Checks' }}
+ if: always()
+ # Reads only the `needs` context, so it needs no token access at all.
+ permissions: {}
+ needs:
+ - preflight
+ - changes
+ - pr_build_linux
+ - pr_build_macos
+ - pr_benchmark_check
+ - spark_3_4
+ - spark_3_5
+ - spark_4_0
+ - spark_4_1
+ - iceberg_1_8
+ - iceberg_1_9
+ - iceberg_1_10
+ - iceberg_1_11
+ runs-on: ubuntu-slim
+ steps:
+ - name: Summarize upstream results
+ env:
+ NEEDS: ${{ toJSON(needs) }}
+ run: echo "$NEEDS"
+
+ # `skipped` is a pass: it means the change did not touch anything that
+ # job covers. Only `failure` and `cancelled` block the merge.
+ - name: Fail if any upstream job failed or was cancelled
+ if: contains(needs.*.result, 'failure') || contains(needs.*.result,
'cancelled')
+ run: |
+ echo "::error::One or more upstream jobs did not succeed. See the
results above."
+ exit 1
diff --git a/.github/workflows/iceberg_spark_test_reusable.yml
b/.github/workflows/iceberg_spark_test_reusable.yml
index 6a268bc129..fe1e4757cd 100644
--- a/.github/workflows/iceberg_spark_test_reusable.yml
+++ b/.github/workflows/iceberg_spark_test_reusable.yml
@@ -162,7 +162,9 @@ jobs:
-Pquick=true -x javadoc
- name: Upload Iceberg shard inventory and test reports
if: ${{ !cancelled() }}
- uses: actions/upload-artifact@v7
+ # iceberg-spark-shard-coverage downloads the inventory, so a flaky
+ # upload here fails that job too. Retry rather than fail on the first
403.
+ uses: ./.github/actions/upload-artifact-retry
with:
name: iceberg-spark-${{ inputs.iceberg-full }}-spark-${{
inputs.spark-full }}-scala-${{ inputs.scala }}-jdk${{ inputs.java }}-shard-${{
matrix.shard }}-attempt-${{ github.run_attempt }}
path: |
diff --git a/.github/workflows/pr_build_linux.yml
b/.github/workflows/pr_build_linux.yml
index ac9e75c5af..58c04f406a 100644
--- a/.github/workflows/pr_build_linux.yml
+++ b/.github/workflows/pr_build_linux.yml
@@ -63,9 +63,28 @@ jobs:
with:
jvm: temurin:21
+ # Split the network from the lint. `cs launch` resolves and downloads the
+ # scalafix artifacts from Maven Central, which resets connections now and
+ # then, so the warm-up retries a no-op `--version` run. The real check
then
+ # runs `--mode offline` against the populated cache: no network, and a
+ # nonzero exit can only mean a lint violation, never a download failure.
+ - name: Fetch scalafix
+ run: |
+ for attempt in 1 2 3; do
+ if cs launch scalafix:0.14.6 -- --version; then
+ break
+ fi
+ if [ "$attempt" = 3 ]; then
+ echo "::error::scalafix download failed after 3 attempts."
+ exit 1
+ fi
+ echo "::warning::scalafix download failed (attempt $attempt of 3);
retrying in $((attempt * 15))s."
+ sleep $((attempt * 15))
+ done
+
- name: Run syntactic scalafix check (no compile)
run: |
- cs launch scalafix:0.14.6 -- \
+ cs launch --mode offline scalafix:0.14.6 -- \
--check \
--syntactic \
--config .scalafix-syntactic.conf \
diff --git a/dev/ci/check-ci-config.py b/dev/ci/check-ci-config.py
index 5b8c9d6e0c..9d5c0b3c48 100644
--- a/dev/ci/check-ci-config.py
+++ b/dev/ci/check-ci-config.py
@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.
-# Guards three CI invariants that are silent when broken:
+# Guards four CI invariants that are silent when broken:
#
# 1. Change-filter routing. dev/ci/compute-changes.py decides which heavy
# jobs run. A file that a job depends on but that no filter lists makes
@@ -28,7 +28,17 @@
# expected sets are transcribed from the `if:` expressions ci.yml carried
# before the policy moved, so a regression here is a behaviour change.
#
-# 3. Artifact-name uniqueness. Artifact names are scoped to the *run*, not
+# 3. Required-check coverage. `Required Checks` in ci.yml is the job that
+# `.asf.yaml` can name in `required_status_checks` for main. A heavy job
+# missing from its `needs:` can fail without blocking the merge, and a
+# rename on either side of the ci.yml/.asf.yaml pair turns the required
+# context into one that never reports, which blocks *every* merge to main
+# until INFRA removes it by hand. The job's name must also route `labeled`
+# runs, which skip the PR tier by design, to a name nothing requires:
+# GitHub keeps the most recent check run per name per commit, so a label
+# run publishing the required name would overwrite the real verdict.
+#
+# 4. Artifact-name uniqueness. Artifact names are scoped to the *run*, not
# to the calling workflow, and ci.yml calls the Spark SQL and Iceberg
# reusable workflows several times in one run. Two producers sharing a
# name make `download-artifact` pick by highest artifact ID rather than
@@ -43,6 +53,23 @@ import sys
from pathlib import Path
WORKFLOWS = Path(".github/workflows")
+ASF_YAML = Path(".asf.yaml")
+
+# The ci.yml job that aggregates every other job's result.
+AGGREGATOR_JOB = "required_checks"
+
+# The aggregator's `name:` has to be this expression shape: label runs publish
+# the first literal, every other event the second. See the comment on the job.
+AGGREGATOR_NAME_EXPR = re.compile(
+ r"^\$\{\{\s*github\.event\.action\s*==\s*'labeled'\s*&&\s*"
+ r"'([^']+)'\s*\|\|\s*'([^']+)'\s*\}\}$"
+)
+
+# Jobs that legitimately stay out of the aggregator's `needs:`. `docs` deploys
+# to asf-site on push to main; it gates nothing and is never part of a merge
+# decision, so folding it in would only turn a failed site deploy into a red
+# `Required Checks` on main.
+AGGREGATOR_EXEMPT = {AGGREGATOR_JOB, "docs"}
# Changed-file list -> the set of outputs compute-changes.py must report true.
# Every other output must be false. Keep one case per shared build input so a
@@ -259,10 +286,185 @@ def check_artifact_names():
return not failures
+def ci_jobs_and_aggregator():
+ """Return (all job ids in ci.yml, aggregator `needs:` ids, aggregator
display name).
+
+ Parsed line by line rather than with PyYAML, which is not installed on the
+ preflight runner.
+ """
+ lines = (WORKFLOWS / "ci.yml").read_text(encoding="utf-8").splitlines()
+ # GitHub job ids may contain letters, digits, `-` and `_`. Matching only
+ # snake_case would let a `spark-4-2:` job escape the coverage check with no
+ # error at all, which is the silent failure this checker exists to catch.
+ job_key = re.compile(r"^ ([A-Za-z0-9_-]+):\s*$")
+ needs_item = re.compile(r"^ - ([A-Za-z0-9_-]+)\s*$")
+ name_key = re.compile(r"^ name:\s*(\S.*?)\s*$")
+
+ jobs, needs, display_name = [], [], None
+ in_jobs = False
+ current = None
+ in_needs_list = False
+ for line in lines:
+ if line.startswith("jobs:"):
+ in_jobs = True
+ continue
+ if not in_jobs:
+ continue
+ match = job_key.match(line)
+ if match:
+ current = match.group(1)
+ jobs.append(current)
+ in_needs_list = False
+ continue
+ if current != AGGREGATOR_JOB:
+ continue
+ if line.strip() == "needs:":
+ in_needs_list = True
+ continue
+ match = name_key.match(line)
+ if match:
+ display_name = match.group(1)
+ match = needs_item.match(line)
+ if in_needs_list and match:
+ needs.append(match.group(1))
+ elif in_needs_list and line.strip() and not line.startswith(" "):
+ in_needs_list = False
+ return jobs, needs, display_name
+
+
+def asf_required_contexts():
+ """Return the required_status_checks contexts .asf.yaml declares for main.
+
+ Scoped to the `main:` entry under `protected_branches:`. Release branches
+ (`branch-0.x`) have their own entries, and a context one of those requires
+ says nothing about what `main` requires.
+ """
+ lines = ASF_YAML.read_text(encoding="utf-8").splitlines()
+ branch_key = re.compile(r"^ ([^\s:#][^:]*):\s*$")
+ context_item = re.compile(r'^\s+- "?(.+?)"?\s*$')
+ contexts = []
+ in_protected = False
+ in_main = False
+ collecting = False
+ for line in lines:
+ stripped = line.strip()
+ if stripped.startswith("#"):
+ continue
+ if stripped == "protected_branches:":
+ in_protected = True
+ continue
+ if not in_protected:
+ continue
+ # A new top-level or `github:`-level key ends the protected_branches
map.
+ if stripped and not line.startswith(" "):
+ break
+ match = branch_key.match(line)
+ if match:
+ in_main = match.group(1) == "main"
+ collecting = False
+ continue
+ if not in_main:
+ continue
+ if stripped == "contexts:":
+ collecting = True
+ continue
+ if not collecting:
+ continue
+ match = context_item.match(line)
+ if match:
+ contexts.append(match.group(1))
+ else:
+ collecting = False
+ return contexts
+
+
+def aggregator_check_names(raw_name):
+ """Split the aggregator's `name:` into (required name, label-run name).
+
+ Returns (None, None) when the value is not the expected expression, which
+ includes a plain literal: a literal name is published on `labeled` runs
too,
+ and that is the failure mode the expression exists to prevent.
+ """
+ match = AGGREGATOR_NAME_EXPR.match(raw_name)
+ if not match:
+ return None, None
+ return match.group(2), match.group(1)
+
+
+def check_required_checks():
+ jobs, needs, display_name = ci_jobs_and_aggregator()
+ failures = []
+
+ if AGGREGATOR_JOB not in jobs:
+ failures.append(
+ f"ci.yml has no `{AGGREGATOR_JOB}` job; it is the only context "
+ f"`.asf.yaml` requires for main"
+ )
+ else:
+ missing = [j for j in jobs if j not in AGGREGATOR_EXEMPT and j not in
needs]
+ if missing:
+ failures.append(
+ f"ci.yml jobs missing from `{AGGREGATOR_JOB}.needs`: "
+ f"{', '.join(missing)}. A job outside the aggregator can fail "
+ f"without blocking the merge queue; add it, or add it to "
+ f"AGGREGATOR_EXEMPT here with a reason"
+ )
+ stale = [n for n in needs if n not in jobs]
+ if stale:
+ failures.append(
+ f"`{AGGREGATOR_JOB}.needs` names jobs that no longer exist in "
+ f"ci.yml: {', '.join(stale)}"
+ )
+ if display_name is None:
+ failures.append(f"the `{AGGREGATOR_JOB}` job in ci.yml has no
`name:`")
+ else:
+ required_name, label_run_name =
aggregator_check_names(display_name)
+ if required_name is None:
+ failures.append(
+ f"`{AGGREGATOR_JOB}.name` is {display_name!r}; it must be
the "
+ f"expression `${{{{ github.event.action == 'labeled' && "
+ f"'<label-run name>' || '<required name>' }}}}`. A label
run "
+ f"skips the PR tier by design, and GitHub keeps only the
most "
+ f"recent check run per name, so publishing the required
name "
+ f"from a label run would overwrite the commit run's
verdict"
+ )
+ elif label_run_name == required_name:
+ failures.append(
+ f"`{AGGREGATOR_JOB}.name` publishes '{required_name}' on "
+ f"label runs too; the `labeled` branch of the expression
must "
+ f"be a different name"
+ )
+ else:
+ # An empty list means main does not require any status check
+ # yet, which is a valid state: there is nothing to keep in
sync.
+ # Once a context is declared, it has to be the one this job
+ # reports on commit runs, and never the label-run name.
+ contexts = asf_required_contexts()
+ if contexts and required_name not in contexts:
+ failures.append(
+ f"`{AGGREGATOR_JOB}` publishes the check name "
+ f"'{required_name}', but .asf.yaml requires {contexts}
"
+ f"for main. A required context that never reports
blocks every "
+ f"merge, including the one that would fix .asf.yaml"
+ )
+ if label_run_name in contexts:
+ failures.append(
+ f".asf.yaml requires '{label_run_name}', which is the
name "
+ f"`{AGGREGATOR_JOB}` publishes only on label runs.
Those runs "
+ f"skip the PR tier, so requiring it would let a label
mark an "
+ f"untested commit green"
+ )
+
+ for failure in failures:
+ print(f"required checks: {failure}")
+ return not failures
+
+
if __name__ == "__main__":
ok = check_change_filters()
ok = check_event_policy() and ok
ok = check_artifact_names() and ok
+ ok = check_required_checks() and ok
if not ok:
sys.exit(1)
print("CI config checks passed")
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
