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-actionsBot 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
rluvatonApr 9, 2026
Copy link
Copy Markdown
MemberAuthor
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-securityAI 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

rluvatonand 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-actionsBot added the common Related to common crate label Apr 9, 2026

rluvatonand 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-actionsBot 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 detectorApr 9, 2026

add coments
10847a0

add comments
e943add

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

mbutrovichand 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 threadci/scripts/changed_crates.sh
Comment threadci/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 73ca6a5Apr 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
Closed

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
Closed

rluvaton mentioned this pull request May 13, 2026
Automate breaking change detection #21406
Closed

Uh oh!

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!

rluvatonApr 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