comphead opened a new pull request, #25443:
URL: https://github.com/apache/datafusion/pull/25443

   ## Which issue does this PR close?
   
   - Part of #25148. This PR does not close the issue.
   - Follow-up to #25365, which did the same for `rust.yml`. This PR answers 
https://github.com/apache/datafusion/pull/25365#issuecomment-5721277119: the 
duplicated check names on the `main` commits page.
   
   ## Rationale for this change
   
   @alamb asked on #25365 why the commits page for `main` shows an inflated 
test count. The checks are duplicated. Every commit is checked twice at the 
same SHA, once by the merge queue and once by the push that follows.
   
   ### Why the same commit is checked twice
   
   The merge queue groups a PR with the tip of `main` onto a temporary branch 
and dispatches `merge_group`. GitHub's docs are explicit that this is a 
distinct event:
   
   > The `merge_group` event is separate from the `pull_request` and `push` 
events.
   >
   > — [Events that trigger workflows: 
`merge_group`](https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#merge_group)
   
   When the group passes, the queue advances `main` to that commit. That fires 
`push`, which "[runs your workflow when you push a commit or 
tag](https://docs.github.com/en/actions/reference/workflows-and-actions/events-that-trigger-workflows#push)".
 Neither event knows the other ran. A workflow listing both therefore runs 
twice at one SHA.
   
   The `branches-ignore: 'gh-readonly-queue/**'` already in both workflows does 
not prevent this. It suppresses a *third* run: the push event on the queue's 
own temporary branch, which is the branch [the docs describe as carrying that 
prefix](https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/managing-a-merge-queue#preparing-for-merge-queues-with-third-party-ci-providers).
 It does nothing about the push to `main`.
   
   Take 3ab72e61e38259f8c52e1937a9c90e7799d7b221. The two `Dev` runs share a 
`head_sha` and differ only in event and ref:
   
   | Run | Event | `head_branch` | `head_sha` |
   | --- | --- | --- | --- |
   | 
[35262885559](https://github.com/apache/datafusion/actions/runs/35262885559) | 
`merge_group` | `gh-readonly-queue/main/pr-25383-98b26c5f9…` | `3ab72e61` |
   | 
[35263735243](https://github.com/apache/datafusion/actions/runs/35263735243) | 
`push` | `main` | `3ab72e61` |
   
   `Dependencies` is the same pair: 
[35262885518](https://github.com/apache/datafusion/actions/runs/35262885518) 
(`merge_group`) and 
[35263735458](https://github.com/apache/datafusion/actions/runs/35263735458) 
(`push`).
   
   That is what the commits page renders. `GET /commits/3ab72e61…/check-runs` 
returns each of these names twice:
   
   ```
   2 success  Check License Header
   2 success  Check Markdown Links
   2 success  Spell Check with Typos
   2 success  Use prettier to check formatting of documents
   2 success  Validate required_status_checks in .asf.yaml
   2 success  Circular Dependency Check
   2 success  Detect Unused Dependencies
   ```
   
   Seven other names are also doubled at that SHA. Those are the `rust.yml` 
jobs #25365 deliberately kept on `main` to save caches, publish coverage, and 
run the non-required FFI check. They are not redundant. The seven above are.
   
   ### Scale
   
   From September 9 to September 17, 2026, `dev.yml` and `dependencies.yml` 
each ran 156 times for pushes to `main`. Every one of those 156 SHAs already 
had a `merge_group` run of the same workflow at the same SHA. The redundancy is 
156/156, not a sample.
   
   Over the seven full days September 10 to 16 there were 120 such commits. 
Across the 40 most recent push runs, `Dev` averaged 1.81 runner-minutes and 
`Dependencies` 4.69, so 6.5 per merged commit, or about 780 runner-minutes per 
week.
   
   All seven jobs report `runner_group: "GitHub Actions"`. None use Runs-On. 
Unlike #25365, whose savings split across GitHub-hosted and ASF AWS capacity, 
every minute saved here comes off the shared ASF GitHub-hosted quota that 
#25148 is about.
   
   ### Why the trigger and not a job condition
   
   `rust.yml` needed a per-job `if:` because seven of its jobs must still run 
post-merge. Neither workflow here has such a job. None uses `actions/cache` or 
`rust-cache`, none uploads an artifact or coverage, and all seven are required 
checks in `.asf.yaml` that the merge queue already satisfies. With nothing to 
keep, excluding the event at the trigger is simpler and strictly cheaper: no 
run is created, so there is no queueing overhead and no skipped-job noise on 
the commit.
   
   Tradeoff: this is ref-based, not repository-based, so a fork pushing to its 
own `main` also stops running these jobs. `extended.yml` already excludes 
`main` at the trigger the same way. Contributors still get all seven on every 
pull request. Happy to switch to the `rust.yml` style `github.repository == 
'apache/datafusion'` guard if reviewers prefer the fork behavior preserved.
   
   ## What changes are included in this PR?
   
   `main` is added to the existing `branches-ignore` list in 
`.github/workflows/dev.yml` and `.github/workflows/dependencies.yml`, with a 
comment explaining why.
   
   Keeping `branches-ignore` rather than switching to a positive `branches:` 
list matters: the merge queue is configured only for `main` in `.asf.yaml`, so 
release branches have no pre-merge run to inherit from and must keep their 
post-push coverage.
   
   | Event | Dev / Dependencies jobs |
   | --- | --: |
   | Push to `main` | 0 |
   | Merge group | 7 |
   | Pull request | 7 |
   | Push to `branch-*` | 7 |
   | Push to any other branch | 7 |
   | `workflow_dispatch` (`Dependencies`) | 2 |
   
   ## What is the testing strategy for this PR?
   
   `python3 ci/scripts/check_asf_yaml_status_checks.py` passes:
   
   ```
   OK: All 35 required_status_checks match existing GitHub Actions jobs.
   OK: rust.yml skips exactly 19 jobs on pushes to main.
   ```
   
   That validator runs as the required `Validate required_status_checks in 
.asf.yaml` job on every PR and merge-group run. It confirms all seven job names 
still resolve and that no required check gained a `pull_request` path filter. 
Its `rust.yml` post-merge assertions are untouched by this change.
   
   Both files were also parsed with `yaml.safe_load` to confirm the trigger 
resolves to `branches-ignore: ['main', 'gh-readonly-queue/**', 
'dependabot/**']`.
   
   No unit test is added. A trigger filter is evaluated by GitHub before a 
runner exists, so there is nothing to assert locally beyond the parse. The 
observable check is the first push to `main` after this merges: `Dev` and 
`Dependencies` should report once at that SHA instead of twice, and the doubled 
names in the list above should disappear from the commits page.
   
   ## Are there any user-facing changes?
   
   No. This PR changes CI only.
   


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to