ci: add breaking change detector by rluvaton · Pull Request #21499 · apache/datafusion

rluvaton · 2026-04-09T09:05:57Z

Which issue does this PR close?

Partially closes:

Automate breaking change detection #21406

Rationale for this change

detect breaking changes

What changes are included in this PR?

add new github workflow

Are these changes tested?

Looks like it is working

Are there any user-facing changes?

no

rluvaton · 2026-04-09T09:13:21Z

Added to here so I can test it

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

rluvaton · 2026-04-09T12:43:28Z

@alamb, @comphead , would love your review
once merge I will iterate over this, but this works from what I tested

rluvaton · 2026-04-27T10:48:44Z

Resolved all comments and verified in my fork:

create breaking changes rluvaton/datafusion#9

…1869) ## Which issue does this PR close? N/A ## Rationale for this change this is done so we can query cargo metadata alone to find only user facing crates and to use cargo metadata to find user facing crates when searching for breaking changes in pr as suggested here: - apache#21499 (comment) ## What changes are included in this PR? set `publish = false` for: 1. `datafusion-benchmarks` 2. `datafusion-wasmtest` 3. `test-utils` ## Are these changes tested? I verified that these are not already published to crates.io and not in `dev/release/README.md` _Publish on Crates.io_ list ## Are there any user-facing changes? No

rluvaton · 2026-04-27T13:32:43Z

@mbutrovich can you please re-review

Jefffrey

Does the check show as a red X when it detects a breaking change?

rluvaton · 2026-04-28T13:41:07Z

Yes

Jefffrey · 2026-04-28T13:50:30Z

Yes

Is it possible to not fail then? Since this is meant to provide a useful comment for reviewers, so having it fail because of a breaking API change being detected could be confusing to PR authors

rluvaton · 2026-04-28T16:22:54Z

done

Jefffrey

I think would be a good idea to try this out for a bit

alamb · 2026-04-29T19:45:00Z

This looks cool -- thank you

This new check is now failing on https://github.com/apache/datafusion/actions/runs/25101909134/job/73651896105?pr=21828

It is not clear from the failure how to resolve the problem (aka how to get the CI to pass after acknowledging the API change)

Is is possible to put some instructions for what to do on failure, for example similar to this:

datafusion/.github/workflows/rust.yml

Lines 66 to 69 in 9a1ed57

    
                     # Adding `--locked` here to assert that the `Cargo.lock` file is up to 
        
                     # date with the manifest. When this fails, please make sure to commit 
        
                     # the changes to `Cargo.lock` after building with the updated manifest. 
        
                     cargo check --profile ci --workspace --all-targets --features integration-tests --locked

?

alamb · 2026-04-29T19:58:49Z

🤔 the failure on this PR seems like a false positive:

feat[expr-common]: support REE in coalesce #21919

This check is failing:

https://github.com/apache/datafusion/actions/runs/25108535879/job/73576369836?pr=21919

martin-g · 2026-04-30T04:52:03Z

The CI checks of #21854 started failing due to this new workflow but the error looks like missing permissions:

existing breaking change comment id 
no comment with breaking changes, creating a new one
gh: Resource not accessible by integration (HTTP 403)
{"message":"Resource not accessible by integration","documentation_url":"https://docs.github.com/rest/issues/comments#create-an-issue-comment","status":"403"}
Error: Process completed with exit code 1.

kumarUjjawal · 2026-04-30T05:05:45Z

There is a pr related to this #21913

…protobuf (apache#21913) ## Which issue does this PR close? - apache#21911 ## Rationale for this change The breaking-change detector added in apache#21499 fails on fork PRs with HTTP 403: > The GITHUB_TOKEN has read-only permissions in pull requests from forked repositories. > > From [GitHub Docs](https://docs.github.com/en/actions/reference/events-that-trigger-workflows#pull_request) A read-only token can't post the sticky comment, so the workflow errors out at the `gh api … POST /comments` call. We can't switch to `pull_request_target` either - ASF infra policy forbids it for any workflow exposing `GITHUB_TOKEN` (https://infra.apache.org/github-actions-policy.html), and `cargo-semver-checks` compiles fork-controlled code (`build.rs`, proc macros) anyway, so granting it a write token would be unsafe. ## What changes are included in this PR? Split the comment posting into a companion `workflow_run` workflow: - `breaking_changes_detector.yml` keeps the `pull_request` trigger but only stages the result (`pr_number`, `result`, `logs`) and uploads it as an artifact. No write token, no comment posting from this workflow. - `breaking_changes_detector_comment.yml` triggers on `workflow_run`, runs in the base-repo context with `pull-requests: write`, downloads the artifact, validates the inputs, and upserts/deletes the sticky comment via `actions-cool/maintain-one-comment`. Never checks out PR code. The comment workflow uses a runtime-randomized heredoc delimiter when piping the fork-controlled logs into `$GITHUB_OUTPUT`, to stop log content from closing the heredoc early and overwriting the validated `result` output (or injecting other keys). Drops the now-unused `comment` subcommand from `ci/scripts/changed_crates.sh`. ---- also installed protobuf as noticed this failed when building subtrait in: - apache#15591 ## Are these changes tested? No, cant really test it ## Are there any user-facing changes? No --------- Co-authored-by: Dmitrii Blaginin <dmitrii@blaginin.me> Co-authored-by: Martin Grigorov <martin-g@users.noreply.github.com>

rluvaton · 2026-04-30T07:01:33Z

The fix is merged, sorry for the trouble

rluvaton added 2 commits April 9, 2026 12:00

ci: add breaking change detector

d128c07

ci: run manually for now

968fc57

github-actions Bot added the development-process Related to development process of DataFusion label Apr 9, 2026

rluvaton changed the title ~~ci: add breaking change detector~~ ci: add breaking change detector (WIP) Apr 9, 2026

add to existing ci job so i can test the workflow

3500fee

rluvaton commented Apr 9, 2026

View reviewed changes

Comment thread .github/workflows/large_files.yml

Copy link
Copy Markdown

Member Author

rluvaton Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added to here so I can test it

github-advanced-security AI found potential problems Apr 9, 2026

View reviewed changes

Comment thread .github/workflows/breaking_changes_detector.yml Fixed

Comment thread .github/workflows/large_files.yml Fixed

rluvaton and others added 9 commits April 9, 2026 12:19

add permission

b0f7cdf

try running

dcb3cfc

fetch main

8ec278a

run on all branches

e15d6a7

run on changed crates

129e180

only if changed

0a863bd

compare against last commit

b3b6dde

test: add temporary public function for semver breaking change testing

95189de

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

test: remove public function to trigger semver breaking change

97ee5b5

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

github-actions Bot added the common Related to common crate label Apr 9, 2026

rluvaton and others added 7 commits April 9, 2026 14:27

fix

d5b1985

test: re-add temporary public function for semver testing

910501c

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

test: remove public function to trigger semver breaking change

f40c07a

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

fix

65779f4

test: re-add temporary public function for semver testing

6de3a14

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

test: remove public function to trigger semver breaking change

1bd00c2

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

revert

b3bf244

rluvaton marked this pull request as ready for review April 9, 2026 12:42

Merge branch 'main' into automate-semver-breaking-api

5286bab

github-actions Bot removed the common Related to common crate label Apr 9, 2026

rluvaton changed the title ~~ci: add breaking change detector (WIP)~~ ci: add breaking change detector Apr 9, 2026

add coments

10847a0

rluvaton added 2 commits April 27, 2026 13:31

fix change detector

7f9d802

add comments

e943add

Merge branch 'main' into automate-semver-breaking-api

2b5e8d4

mbutrovich and others added 2 commits April 27, 2026 09:58

Merge branch 'main' into automate-semver-breaking-api

e9654b2

Merge branch 'main' into automate-semver-breaking-api

31dade4

Jefffrey reviewed Apr 28, 2026

View reviewed changes

Comment thread .github/workflows/breaking_changes_detector.yml Outdated

Comment thread .github/workflows/breaking_changes_detector.yml Outdated

Comment thread ci/scripts/changed_crates.sh

Comment thread ci/scripts/changed_crates.sh Outdated

rluvaton added 2 commits April 28, 2026 19:21

address review feedback

1077e92

Merge branch 'main' into automate-semver-breaking-api

fbb2566

Jefffrey approved these changes Apr 29, 2026

View reviewed changes

rluvaton added this pull request to the merge queue Apr 29, 2026

Merged via the queue into apache:main with commit 73ca6a5 Apr 29, 2026
34 checks passed

rluvaton deleted the automate-semver-breaking-api branch April 29, 2026 06:58

rluvaton mentioned this pull request Apr 29, 2026

build: allow posting comments on PRs made from forks and fix missing protobuf #21913

Merged

alamb mentioned this pull request Apr 29, 2026

feat: pushdown OFFSET to parquet for RG-level skipping #21828

Open

This was referenced Apr 29, 2026

feat[expr-common]: support REE in coalesce #21919

Merged

test(sqllogictest): stabilize parquet output_rows_skew with WITH ORDER #21898

Merged

rluvaton mentioned this pull request Apr 30, 2026

Breaking change detector: adding "auto detect api change" label on detection #21952

Open

Conversation

rluvaton commented Apr 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

rluvaton Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rluvaton commented Apr 9, 2026

Uh oh!

rluvaton commented Apr 27, 2026

Uh oh!

rluvaton commented Apr 27, 2026

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rluvaton commented Apr 28, 2026

Uh oh!

Jefffrey commented Apr 28, 2026

Uh oh!

rluvaton commented Apr 28, 2026

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alamb commented Apr 29, 2026

Uh oh!

alamb commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martin-g commented Apr 30, 2026

Uh oh!

kumarUjjawal commented Apr 30, 2026

Uh oh!

rluvaton commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

rluvaton commented Apr 9, 2026 •

edited

Loading

alamb commented Apr 29, 2026 •

edited

Loading