You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/09/10 18:14:03 UTC
[PR] ci: move the Spark 3.4/3.5/4.0 suites behind a merge queue [datafusion-comet]
andygrove opened a new pull request, #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843
## Which issue does this PR close?
Closes #5838.
## Rationale for this change
CI load on pull requests is dominated by suites that don't really need to be green until merge time. A merge queue lets us move them there: the queue tests the exact tree that will land, so an iterating PR can run a smaller set without losing any coverage on `main`.
The part I expected to be a blocker turned out not to be. Merge queue is self-serve in `.asf.yaml` now — rulesets there accept a raw GitHub Rulesets API payload and `asfyaml/feature/github/rulesets.py` passes it through with no rule-type allowlist, so a `merge_queue` rule needs no INFRA ticket. apache/iceberg-python, apache/seatunnel and apache/texera all run queues configured this way. Worth noting apache/datafusion is _not_ the template even though it's the obvious place to look: their `.asf.yaml` has no queue config and `gh api repos/apache/datafusion/rulesets` returns `[]`, because INFRA enabled theirs directly in classic branch protection via INFRA-27154 before asfyaml supported rulesets.
## What changes are included in this PR?
CI splits into two tiers:
| | PR tier | queue tier | push to main |
| --- | --- | --- | --- |
| build Linux/macOS, benchmark, Spark 4.1, Iceberg 1.11 | ✓ | ✓ | — |
| Spark 3.4 / 3.5 / 4.0, Iceberg 1.8 / 1.9 / 1.10 | label only | ✓ | — |
| docs deploy | — | — | ✓ |
Spark 3.5 is the one that changes for contributors: it ran on every PR and is now queue-only, with a `run-spark-3.5-tests` label as an escape hatch. Spark 4.1 stays in the PR tier because it's the default build profile, so it's the cheapest early warning that something is wrong.
Dropping the `push` arm from the heavy jobs is load-bearing rather than cleanup. The queue already tested the tree that lands, so keeping the old push-to-main runs would make every merge pay for CI twice and total usage would go *up*, not down. Iceberg 1.8/1.9/1.10 move into the queue for the same reason — they were push-only, so otherwise they'd have nowhere left to run.
On the `.asf.yaml` side: a `Merge Queue` ruleset targeting `~DEFAULT_BRANCH` (merge queue rules reject wildcard ref patterns, so release branches keep plain protection), `Required Checks` as the single required status check, and `allow_auto_merge` so a PR can be queued before its checks finish. `apache/root` is a bypass actor so a wedged queue can be recovered without a ticket. GitHub evaluates the ruleset and classic branch protection together, so the existing one-approving-review requirement is unaffected.
The queue parameters worth arguing about are `max_entries_to_merge: 5` with `max_entries_to_build: 2` and no minimum wait — no added latency when the queue is empty, but once it backs up a single pipeline validates up to five PRs. Easy to tune later.
## How are these changes tested?
`preflight` runs `dev/ci/check-ci-config.py`, which (as of the previous commit) enforces that every job is registered with the aggregator and that the `Required Checks` name matches the context this PR adds to `.asf.yaml`. `actionlint`, `prettier --check "**/*.md"` and a YAML parse of both changed configs are clean.
The `merge_group` paths can't really be exercised until the ruleset is live, which is the main reason this is split out from #5842 — that PR lands `Required Checks` on its own so we can confirm it reports correctly on real PRs first.
## Notes for reviewers
- Stacked on top of the `Required Checks` PR, so it shows both commits until that one merges. **Do not merge this before it.**
- `run-spark-3.4-tests` and `run-spark-3.5-tests` need creating in repository settings. 3.4's label doesn't exist today either, so that escape hatch has quietly been doing nothing.
- Once these suites gate the queue, a flaky test blocks everyone's merges rather than one PR. datafusion sent a dev@ note before enabling theirs and we should probably do the same.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843#issuecomment-5640642251
CI failure is unrelated:
```
- fall-back: oss data location scheme (oss.* properties are not forwarded) (92 milliseconds)
#
# A fatal error has been detected by the Java Runtime Environment:
#
# SIGSEGV (0xb) at pc=0x00007fb9d00e5800, pid=1201, tid=4733
#
# JRE version: OpenJDK Runtime Environment Zulu11.90+19-CA (11.0.32+9) (build 11.0.32+9-LTS)
# Java VM: OpenJDK 64-Bit Server VM Zulu11.90+19-CA (11.0.32+9-LTS, mixed mode, tiered, compressed oops, g1 gc, linux-amd64)
# Problematic frame:
# C 0x00007fb9d00e5800
#
# Core dump will be written. Default location: Core dumps may be processed with "/usr/lib/systemd/systemd-coredump %P %u %g %s %t 9223372036854775808 %h %d" (or dumping to /__w/datafusion-comet/datafusion-comet/spark/core.1201)
#
# An error report file with more information is saved as:
# /__w/datafusion-comet/datafusion-comet/spark/hs_err_pid1201.log
#
# If you would like to submit a bug report, please visit:
# http://www.azul.com/support/
#
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843#discussion_r3992866052
##########
.github/actions/download-artifact-retry/action.yaml:
##########
@@ -0,0 +1,107 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: "Download Artifact (with retry)"
+description: >
+ Drop-in replacement for actions/download-artifact that retries the download
Review Comment:
Fixed in 049fa835. `.github/actions/download-artifact-retry/**` now sits in the same ten filters that already listed the upload wrapper, with the matching `ROUTING_CASES` entry beside it. Dropping it from one filter fails the check with `expected iceberg_1_11=true, got false`, so it can't quietly go missing again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue [datafusion-comet]
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on code in PR #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843#discussion_r3991078036
##########
.github/actions/download-artifact-retry/action.yaml:
##########
@@ -0,0 +1,107 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+name: "Download Artifact (with retry)"
+description: >
+ Drop-in replacement for actions/download-artifact that retries the download
Review Comment:
### Correctness
[P2] Register the new shared action in the consuming path filters. `FILTERS` lists `upload-artifact-retry/**` but never `download-artifact-retry/**`. For an edit confined to this new action, the current `compute()` returns every output false on both a PR update and `merge_group`, so none of its Linux/macOS/Spark/Iceberg consumers exercise the change. The existing config guard also passes that configuration. Add this path to the ten consuming filters and a corresponding `ROUTING_CASES` entry, as for the upload wrapper. I verified that adding only those dependencies in memory restores all ten queue routes.
##########
.github/workflows/spark_sql_test_reusable.yml:
##########
@@ -285,7 +285,9 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: Download fallback log artifacts
- uses: actions/download-artifact@v8
+ # Not wrapped in download-artifact-retry: that local action needs a
+ # checkout, which this job deliberately skips.
+ uses: ./.github/actions/download-artifact-retry
Review Comment:
### Correctness
[P2] Keep this download usable without a checkout. When a caller enables `collect-fallback-logs`, this job starts on its own runner and its first step now references `./.github/actions/download-artifact-retry`, but no step has checked out that directory. The action cannot be loaded, so the merged fallback-log artifact is no longer produced. The preceding comment describes the intended exception, but the `uses` value does the opposite. Restore `actions/download-artifact@v8` here, or add a checkout before the local action. Current umbrella callers leave the option false, which explains why ordinary CI does not exercise this regression. [GitHub's local-action requirement](https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax#example-using-an-action-in-the-same-repository-as-the-workflow)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843#discussion_r3990747929
##########
dev/ci/compute-changes.py:
##########
@@ -301,28 +301,43 @@
# Which events may run each job, independent of the path filters above.
#
# "pr" every pull request
+# "queue" the merge queue, i.e. a merge_group event
# "push" push to main
# "label:<name>" a pull request carrying that label
#
# workflow_dispatch always runs everything, so it is not listed. "pr" and
# "label:" are mutually exclusive -- a job is either unconditional on pull
# requests or opt-in, never both -- and check-ci-config.py rejects a job that
# lists both rather than letting the label quietly win.
+#
+# Almost everything is "queue": the merge queue is the authoritative gate, and
+# it tests the merge result rather than the PR head. "push" is reserved for
+# work that can only happen once a commit is on main. Adding "push" back to a
+# test job would make every merge run it twice, once in the queue and once
+# after, which is the thing the queue was adopted to avoid.
POLICY = {
- "build_linux": ["pr", "push"],
- "build_macos": ["pr", "push"],
- "benchmark": ["pr", "push"],
- # docs deploys to asf-site, so it must not run from a pull request.
+ "build_linux": ["pr", "queue"],
Review Comment:
Thanks @blaginin. I went ahead and addressed this in the current PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843#issuecomment-5640612473
I checked the ASF side of this against the asfyaml source and GitHub's merge queue docs, and read the workflows at `6cacd83`. The claims that would sink it all hold up:
- `asfyaml/feature/github/rulesets.py` forwards a raw `rules` list to GitHub untouched. The only rule-type inspection is the `copilot_code_review` overlap check, so `merge_queue` needs no INFRA ticket.
- It coerces exactly the merge-queue numeric keys plus `actor_id` (INFRA-27864). It *silently leaves* a non-integer alone, so a typo surfaces as a GitHub 422 rather than an asfyaml error.
- `apache/root` is team id `118420`. The repo currently has zero rulesets, so `reconcile_rulesets` creates `Merge Queue` cleanly.
- `merge_method: SQUASH` matches `enabled_merge_buttons`.
- `apache/iceberg-python` is real prior art, tuned much more conservatively: `max_entries_to_build: 1`, `max_entries_to_merge: 1`, `check_response_timeout_minutes: 90`, no bypass actors.
## Rollout risk
**Rehearse `merge_group` on a scratch branch first.** The path has never executed. If `changes` errors on a merge group under `set -euo pipefail`, `required_checks` goes red, every entry is evicted, and the fix has to merge through the broken queue. This is self-serve: land a ruleset targeting a literal `refs/heads/mq-rehearsal` with both `merge_queue` and `required_status_checks: Required Checks`. Merge-queue rules reject wildcards, not literal refs, and `ci.yml` has no `branches:` filter on `pull_request` or `merge_group`, so a throwaway PR against that branch exercises the whole path. A ruleset on a scratch branch cannot wedge `main`.
**Make the merge_group diff fail open**, as cheap insurance:
```bash
git diff --name-only "$MQ_BASE_SHA"..."$MQ_HEAD_SHA" > changed_files.txt || {
echo "::warning::merge_group diff failed; running everything"
git ls-tree -r --name-only "$MQ_HEAD_SHA" > changed_files.txt
}
```
**Add a project-side bypass actor.** `apache/root` still means an INFRA ticket. `apache/datafusion-committers` exists (id `9960695`) and would let the PMC merge the fix directly. Ruleset bypass does not bypass classic branch protection, so the one-approval requirement still applies.
## Gaps in the diff
**The `actions/cache` exception was applied to Linux only.** `build_linux` was kept on `push` for cache freshness, but it is not the only job writing caches nothing else writes:
| Cache key | Written at | Only writer | Push tier after this PR |
| --- | --- | --- | --- |
| `macOS-cargo-ci-v2-*` | `pr_build_macos.yml:89` | `build_macos` | gone |
| `macOS-cargo-registry-*` | `pr_build_macos.yml:280` | `build_macos` | gone |
| `macOS-java-maven-*` | `actions/java-test/action.yaml:70` | `build_macos` | gone |
| `Linux-spark-sql-*` | `actions/setup-spark-builder/action.yaml:55` | `spark_sql_test_reusable.yml` | gone from all four callers |
| `Linux-benchmark-maven-*` | `pr_benchmark_check.yml:63` | `benchmark` | gone |
The explicit saves are guarded on `github.ref == 'refs/heads/main'`, false on a `gh-readonly-queue/*` branch, and the `actions/cache@vN` auto-saves land in a queue scope deleted with the branch. `restore-keys` means gradual degradation rather than a cliff, but `Linux-spark-sql-*` is the `~/.m2/repository` for the suites now on the critical path of every merge, and a cold macOS cargo build on every queue run works against `max_entries_to_build`. Options: accept it and say so in `POLICY`, add `"push"` back to `spark_4_1` on the same grounds, or add a push-only cache-warm macOS job without the test matrix.
**In the queue, a fork PR's code runs with a write-capable `GITHUB_TOKEN`.** A `merge_group` run is on a base-repo branch, so it gets the repo default token permissions and repository secrets, and `actions/checkout` persists that token into `.git/config`. Under `pull_request` those same suites get a read-only token and no secrets. `docs.yaml` pushes to `asf-site` with no `permissions:` block, which only works if the repo default is read and write, and neither `ci.yml` nor any reusable workflow it calls declares one. Cargo build scripts, Maven/sbt plugins and the applied Spark diff all run in that context. Fix is small, and also clears the CodeQL finding you hit on `required_checks`:
```yaml
permissions:
contents: read
jobs:
docs:
permissions:
contents: write # pushes to asf-site
```
**Four workflows sit outside the umbrella and outside the queue.** `codeql.yml`, `delta_build_gate.yml` and `pyarrow_udf_test.yml` trigger on `pull_request` and `push: main` with no `merge_group`. They are never evaluated against the merge result, are not in `required_checks.needs`, and are not required, so a red one does not stop a queueing. They also still run on `push`, which contradicts the no-push-tier rule stated two files away. Either fold them into `ci.yml` as reusables so they inherit the aggregator, or document them as advisory. `check-ci-config.py` only reasons about jobs inside `ci.yml`, so a new top-level workflow escapes the gate silently.
## Tuning
**`check_response_timeout_minutes: 300` is calibrated on the PR tier.** The queue tier adds three Spark SQL suites (each with its own ~40 minute build job), three Iceberg suites, macOS and the benchmark check. At `max_entries_to_build: 2` that is roughly 150 concurrent jobs from this repo against a shared ASF pool, so wall clock is set by scheduling delay rather than fan-out. You can measure it without the queue: apply all six opt-in labels and then push a commit, since on `synchronize` the gates read from `PR_LABELS` and one run reproduces the queue tier exactly. Not `workflow_dispatch` though, since `event_allows` returns `True` unconditionally for it including for `docs`, and `docs.yaml` pushes to `asf-site` with no ref guard.
**The cost model turns on the rebuild rate, not the tier sizes.** Per merged PR with `P` CI-triggering pushes and `B` merge-group builds charged to it:
```
Δ = P × (macOS + benchmark + Spark 3.5) − (B − 1) × FULL − Linux-on-push
```
A full pipeline is several times `macOS + benchmark + Spark 3.5`, so every extra build charged to an entry eats several pushes' worth of savings. Probably still net negative at a typical `P`, but the margin is governed by `B`, so tracking evictions per week from day one is what decides whether this worked.
**`ALLGREEN` is the right default, and there is a stronger argument for it than the comment gives.** Under `HEADGREEN` with `SQUASH`, a group of two lands two commits on `main` and the first was never green on its own, which breaks bisect. The counterweight is that GitHub explicitly recommends `HEADGREEN` for flaky CI, and this repo has enough infra flakiness to justify retry wrappers on both upload and download. Worth naming the flip condition in the `.asf.yaml` comment, for example an eviction rate threshold over a week. One-line change either way.
## Minor
- `allow_update_branch: true` becomes mostly redundant once the queue is live, and each click costs a full PR-tier rerun.
- Do not add `strict: true` later. Requiring branches to be up to date fights the queue.
- `check-ci-config.py` could assert `merge_method` matches the single enabled entry in `enabled_merge_buttons`, since drift there breaks every merge.
- The `build_linux` comment says queue caches "are deleted with the branch". For the three explicit `cache/save` steps the queue never writes them at all, because of the `refs/heads/main` guard. Same outcome, different mechanism.
- Agree on the dev@ note. That is also where the eviction-and-requeue recovery path should be spelled out for contributors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843#issuecomment-5639868926
Thanks @sunchao. You're right about the queue parameters, and it's worse than a wording problem — GitHub says outright that merge limits do not combine `merge_group` builds, so `max_entries_to_merge: 5` saves no CI whatsoever. I had the cost model backwards. `.asf.yaml`, the workflows README and the PR description all asserted the batching; all three are corrected in 6cacd83.
While rewriting those comments I noticed the `ALLGREEN` one was describing HEADGREEN's behaviour — "a batch is only merged if the combined tree is green" is the green-head rule, not the all-green one. Fixed that too.
The correction also moves where the real constraint sits. With every entry getting its own build, `max_entries_to_build: 2` is the throughput ceiling rather than a speculation-cost knob: two pipelines at the ~2.5h we see today drains roughly 19 merges a day. Over the last 30 days `main` took a median of 6 merges a day but 22 on the busiest, so 2 would have been the bottleneck on the worst day. I've left it at 2 and written those numbers into the comment rather than changing the value, since raising it costs that many concurrent ASF runners and that felt like a call to make deliberately rather than fold into this PR. Happy to start at 3 if you think that's the safer default.
The case for the split itself is unaffected: the saving comes from the PR tier being small, not from anything the queue batches.
And agreed on the rollout caveat — the ruleset and the `merge_group` paths are proposed configuration until it's live, which is why #5842 landed `Required Checks` on its own first so we could at least confirm the required context reports correctly on real PRs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on code in PR #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843#discussion_r3992867446
##########
.github/workflows/spark_sql_test_reusable.yml:
##########
@@ -285,7 +285,9 @@ jobs:
runs-on: ubuntu-24.04
steps:
- name: Download fallback log artifacts
- uses: actions/download-artifact@v8
+ # Not wrapped in download-artifact-retry: that local action needs a
+ # checkout, which this job deliberately skips.
+ uses: ./.github/actions/download-artifact-retry
Review Comment:
Good catch, and the worst combination: the comment was right and the `uses:` was wrong. Restored `actions/download-artifact@v8` in f36626df.
Since no caller sets `collect-fallback-logs`, a comment was never going to hold that line, so I also added a fifth invariant to `check-ci-config.py` — any `uses: ./.github/actions/...` in a workflow needs an `actions/checkout` earlier in the same job. Reintroducing the bad `uses:` now fails preflight naming exactly that step.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: move the Spark 3.4/3.5/4.0 suites behind a merge queue [datafusion-comet]
Posted by "github-advanced-security[bot] (via GitHub)" <gi...@apache.org>.
github-advanced-security[bot] commented on code in PR #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843#discussion_r3982165564
##########
.github/workflows/ci.yml:
##########
@@ -367,3 +403,56 @@
spark-short: '4.1'
spark-full: '4.1.3'
java: 17
+
+ # ---------------------------------------------------------------------------
+ # required_checks: the single context listed in `required_status_checks` for
+ # `main` in `.asf.yaml`, and therefore the only thing the merge queue waits
+ # on.
+ #
+ # 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.
+ # ---------------------------------------------------------------------------
+ required_checks:
+ name: Required Checks
+ if: always()
+ 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
Review Comment:
## CodeQL / Workflow does not contain permissions
Actions job or workflow does not limit the permissions of the GITHUB_TOKEN. Consider setting an explicit permissions block, using the following as a minimal starting point: {{}}
[Show more details](https://github.com/apache/datafusion-comet/security/code-scanning/147)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove merged PR #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue [datafusion-comet]
Posted by "blaginin (via GitHub)" <gi...@apache.org>.
blaginin commented on code in PR #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843#discussion_r3990171678
##########
dev/ci/compute-changes.py:
##########
@@ -301,28 +301,43 @@
# Which events may run each job, independent of the path filters above.
#
# "pr" every pull request
+# "queue" the merge queue, i.e. a merge_group event
# "push" push to main
# "label:<name>" a pull request carrying that label
#
# workflow_dispatch always runs everything, so it is not listed. "pr" and
# "label:" are mutually exclusive -- a job is either unconditional on pull
# requests or opt-in, never both -- and check-ci-config.py rejects a job that
# lists both rather than letting the label quietly win.
+#
+# Almost everything is "queue": the merge queue is the authoritative gate, and
+# it tests the merge result rather than the PR head. "push" is reserved for
+# work that can only happen once a commit is on main. Adding "push" back to a
+# test job would make every merge run it twice, once in the queue and once
+# after, which is the thing the queue was adopted to avoid.
POLICY = {
- "build_linux": ["pr", "push"],
- "build_macos": ["pr", "push"],
- "benchmark": ["pr", "push"],
- # docs deploys to asf-site, so it must not run from a pull request.
+ "build_linux": ["pr", "queue"],
Review Comment:
(for a pr on top) - let's run this on push to `main`? So will keep updating the cache...
<img width="1208" height="260" alt="Image" src="https://github.com/user-attachments/assets/e7fe1928-c24b-43c1-b352-e5b3ec4ac63e" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] ci: move the Spark 3.4/3.5/4.0, Iceberg, macOS and benchmark suites behind a merge queue [datafusion-comet]
Posted by "comphead (via GitHub)" <gi...@apache.org>.
comphead commented on PR #5843:
URL: https://github.com/apache/datafusion-comet/pull/5843#issuecomment-5636158277
Thanks @andygrove I'll check this today
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org