Skip to content

Decouple PR comment workflow from semver check and install protoc for cargo-semver-checks#21951

Closed
kosiew wants to merge 2 commits intoapache:mainfrom
kosiew:workflow-21914
Closed

Decouple PR comment workflow from semver check and install protoc for cargo-semver-checks#21951
kosiew wants to merge 2 commits intoapache:mainfrom
kosiew:workflow-21914

Conversation

@kosiew
Copy link
Copy Markdown
Contributor

@kosiew kosiew commented Apr 30, 2026

Which issue does this PR close?


Rationale for this change

The existing PR comment step was part of the pull_request workflow, which caused failures for forked PRs due to restricted GITHUB_TOKEN permissions (read-only). This resulted in 403 Resource not accessible by integration errors when attempting to post comments. I encountered this while working on #21917.

image

Additionally, updates to Cargo metadata can trigger cargo-semver-checks to build with all features enabled, pulling in dependencies (e.g., Substrait) that require protoc. Without installing protoc, the semver check environment may fail to build.

This change improves both reliability and security by ensuring the workflow environment matches build requirements and by running comment updates in a context with appropriate permissions.


What changes are included in this PR?

  • Introduced a new workflow_run-based workflow to handle PR comment updates after the semver check completes.
  • Removed the inline PR comment step from the main pull_request workflow.
  • Added logic to persist semver check results and logs as an artifact.
  • Downloaded and validated semver results in the follow-up workflow before posting comments.
  • Derived the PR number from the trusted workflow_run payload instead of storing it in artifacts.
  • Installed protoc in the semver check job when needed to support builds requiring protobuf.
  • Ensured unexpected semver results fail explicitly.

Are these changes tested?

No additional tests are included. These changes affect CI workflows and are validated through workflow execution.


Are there any user-facing changes?

No user-facing changes. These updates only affect CI behavior and internal workflow execution.


LLM-generated code disclosure

This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed and tested.

kosiew added 2 commits April 30, 2026 12:45
…er and improve artifact handling

- Install protobuf-compiler before cargo-semver-checks in the check-semver job.
- Store semver results, PR number, and logs in an artifact instead of commenting directly on the PR.
- Change artifact handoff to default to success only for the "no changed crates" case.
- Add breaking_changes_comment.yml to manage PR comments based on workflow_run triggers.
- Ensure comment workflow is gated to successful upstream runs, preventing comments on cancelled or failed detector runs.
- Update `.github/workflows/breaking_changes_comment.yml` to derive `PR_NUMBER` from `github.event.workflow_run.pull_requests[0].number` and remove reliance on `pr_number.txt`. Added validation for `CHECK_RESULT` to ensure it is one of `success`, `failure`, or `error`.
- Modify `.github/workflows/breaking_changes_detector.yml` to stop writing `pr_number.txt` into the artifact.
@github-actions github-actions Bot added the development-process Related to development process of DataFusion label Apr 30, 2026
@kosiew kosiew requested a review from alamb April 30, 2026 05:23
@kosiew kosiew marked this pull request as ready for review April 30, 2026 05:23
@martin-g
Copy link
Copy Markdown
Member

Duplicate of #21913

@martin-g martin-g marked this as a duplicate of #21913 Apr 30, 2026
@rluvaton
Copy link
Copy Markdown
Member

Closing as this was fixed, sorry for the trouble

@rluvaton rluvaton closed this Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

development-process Related to development process of DataFusion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants