Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run RevAPI without Gradle #10368

Closed
Fokko opened this issue May 23, 2024 · 10 comments
Closed

Run RevAPI without Gradle #10368

Fokko opened this issue May 23, 2024 · 10 comments
Assignees
Labels
improvement PR that improves existing functionality

Comments

@Fokko
Copy link
Contributor

Fokko commented May 23, 2024

Feature Request / Improvement

By decoupling RevAPI from Gradle we can upgrade to a later version of Gradle.

A later version of Gradle is required to unblock #10209. The 1.14.0 version of Parquet ships with a newer version of Jackson, with JDK21-specific files. These files cause issues with the older version of the Shadow plugin that we're using. The shadow plugin requires a later version of Gradle.

We must double-check if RevAPI is working properly since it silently stopped working when updating Gradle to a later version.

Blocks #8485

I believe @jbonofre wants to work on this, but he can also work on it if anyone else is interested.

Query engine

None

@Fokko Fokko added the improvement PR that improves existing functionality label May 23, 2024
@jbonofre
Copy link
Member

Yes I have something about that coupled with gradle update. I will push the PR asap.

@jbonofre
Copy link
Member

I created a script to run revapi: https://github.com/jbonofre/iceberg/blob/GRADLE/dev/revapi

I have three things to address:

  1. support acceptedBreaks from .palantir/revapi.yml (I might need more than a script for that), I'm checking different options
  2. the script should failed in error when revapi detects a breaking change
  3. integrate the script in gradle

@jbonofre
Copy link
Member

After some investigations, I'm doing:

  1. I'm replacing .palantir/revapi.yml (specific to the revapi gradle plugin) by a simple file where users can add accepted breaks
  2. the script now detects a breaking change by grepping the output report
  3. I'm not sure if the script should be integrated in gradle or not. Probably worth to execute the script after the build (the script needs to be run after compileJava task).

@findepi
Copy link
Member

findepi commented Jun 11, 2024

How does revapi manifests that it does not support newer gradle?

@jbonofre
Copy link
Member

@findepi if you change a public API (let's say SessionCatalog and RESTSessionCatalog, adding a new method there), with Gradle 8.2+, no error, whereas it should have detected a breaking change on the public API.

I'm working on the fix.

@findepi
Copy link
Member

findepi commented Jun 12, 2024

@jbonofre do we already know the root cause why the rev api stops working when we upgrade grade? If it something not our side, are the plug-in authors aware of the problem?

@ajantha-bhat
Copy link
Member

@findepi: The problem is gradle plugin is not officially part of rev API maintainers.
It is a separate group and they stopped maintaining it (last release was 2022).

We also created the issue but no one responded
palantir/gradle-revapi#612

@jbonofre
Copy link
Member

@findepi yes I have the fix for the plugin. But my concern is the plugin is not maintained anymore (no commit or release in the last 2 years).

@ajantha-bhat
Copy link
Member

I started this discussion also: revapi/revapi#296

@Fokko
Copy link
Contributor Author

Fokko commented Jun 25, 2024

Not needed anymore #8486

@Fokko Fokko closed this as completed Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement PR that improves existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants