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

GH-40878: [JAVA] Fix flight-sql-jdbc-driver shading issues #40879

Merged

Conversation

laurentgo
Copy link
Collaborator

@laurentgo laurentgo commented Mar 29, 2024

Rationale for this change

The flight-sql-jdbc-driver jar is not shaded properly:

  • a reduced pom.xml file is not generated. The published pom.xml file declares dependencies which are actually present in the jar and should not be fetched externally
  • several classes/files are not relocated properly

What changes are included in this PR?

Fix pom.xml and relocations. Also removes annotations dependencies and include a integration test to prevent future breakage.

Are these changes tested?

Yes. A new integration test check the jar content

Are there any user-facing changes?

Yes. The published pom.xml file on Maven will be cleaned of any dependency

The `flight-sql-jdbc-driver` jar is not shaded properly:
* a reduced pom.xml file is not generated. The published pom.xml file
  declares dependencies which are actually present in the jar and should
  not be fetched externally
* several classes/files are not relocated properly

Fix pom.xml and relocations. Also removes annotations dependencies and
include a integration test to prevent future breakage.
Copy link

⚠️ GitHub issue #40878 has been automatically assigned in GitHub to PR creator.

Copy link
Member

@jbonofre jbonofre left a comment

Choose a reason for hiding this comment

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

LGTM. Shading simplifies driver use :)

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Mar 29, 2024
Copy link
Member

@lidavidm lidavidm left a comment

Choose a reason for hiding this comment

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

Thank you!

@github-actions github-actions bot added awaiting merge Awaiting merge and removed awaiting committer review Awaiting committer review labels Mar 29, 2024
@lidavidm
Copy link
Member

@github-actions crossbow submit java

Copy link

Revision: 91973e9

Submitted crossbow builds: ursacomputing/crossbow @ actions-b86db249a8

Task Status
java-jars GitHub Actions
verify-rc-source-java-linux-almalinux-8-amd64 GitHub Actions
verify-rc-source-java-linux-conda-latest-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-20.04-amd64 GitHub Actions
verify-rc-source-java-linux-ubuntu-22.04-amd64 GitHub Actions
verify-rc-source-java-macos-amd64 GitHub Actions

@lidavidm lidavidm merged commit 9f0101e into apache:main Mar 30, 2024
16 checks passed
@lidavidm lidavidm removed the awaiting merge Awaiting merge label Mar 30, 2024
Copy link

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 9f0101e.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 1 possible false positive for unstable benchmarks that are known to sometimes produce them.

tolleybot pushed a commit to tmct/arrow that referenced this pull request May 2, 2024
…che#40879)

### Rationale for this change

The `flight-sql-jdbc-driver` jar is not shaded properly:
* a reduced pom.xml file is not generated. The published pom.xml file declares dependencies which are actually present in the jar and should not be fetched externally
* several classes/files are not relocated properly

### What changes are included in this PR?

Fix pom.xml and relocations. Also removes annotations dependencies and include a integration test to prevent future breakage.

### Are these changes tested?

Yes. A new integration test check the jar content

### Are there any user-facing changes?

Yes. The published pom.xml file on Maven will be cleaned of any dependency
* GitHub Issue: apache#40878

Authored-by: Laurent Goujon <[email protected]>
Signed-off-by: David Li <[email protected]>
tolleybot pushed a commit to tmct/arrow that referenced this pull request May 4, 2024
…che#40879)

### Rationale for this change

The `flight-sql-jdbc-driver` jar is not shaded properly:
* a reduced pom.xml file is not generated. The published pom.xml file declares dependencies which are actually present in the jar and should not be fetched externally
* several classes/files are not relocated properly

### What changes are included in this PR?

Fix pom.xml and relocations. Also removes annotations dependencies and include a integration test to prevent future breakage.

### Are these changes tested?

Yes. A new integration test check the jar content

### Are there any user-facing changes?

Yes. The published pom.xml file on Maven will be cleaned of any dependency
* GitHub Issue: apache#40878

Authored-by: Laurent Goujon <[email protected]>
Signed-off-by: David Li <[email protected]>
rok pushed a commit to tmct/arrow that referenced this pull request May 8, 2024
…che#40879)

### Rationale for this change

The `flight-sql-jdbc-driver` jar is not shaded properly:
* a reduced pom.xml file is not generated. The published pom.xml file declares dependencies which are actually present in the jar and should not be fetched externally
* several classes/files are not relocated properly

### What changes are included in this PR?

Fix pom.xml and relocations. Also removes annotations dependencies and include a integration test to prevent future breakage.

### Are these changes tested?

Yes. A new integration test check the jar content

### Are there any user-facing changes?

Yes. The published pom.xml file on Maven will be cleaned of any dependency
* GitHub Issue: apache#40878

Authored-by: Laurent Goujon <[email protected]>
Signed-off-by: David Li <[email protected]>
rok pushed a commit to tmct/arrow that referenced this pull request May 8, 2024
…che#40879)

### Rationale for this change

The `flight-sql-jdbc-driver` jar is not shaded properly:
* a reduced pom.xml file is not generated. The published pom.xml file declares dependencies which are actually present in the jar and should not be fetched externally
* several classes/files are not relocated properly

### What changes are included in this PR?

Fix pom.xml and relocations. Also removes annotations dependencies and include a integration test to prevent future breakage.

### Are these changes tested?

Yes. A new integration test check the jar content

### Are there any user-facing changes?

Yes. The published pom.xml file on Maven will be cleaned of any dependency
* GitHub Issue: apache#40878

Authored-by: Laurent Goujon <[email protected]>
Signed-off-by: David Li <[email protected]>
vibhatha pushed a commit to vibhatha/arrow that referenced this pull request May 25, 2024
…che#40879)

### Rationale for this change

The `flight-sql-jdbc-driver` jar is not shaded properly:
* a reduced pom.xml file is not generated. The published pom.xml file declares dependencies which are actually present in the jar and should not be fetched externally
* several classes/files are not relocated properly

### What changes are included in this PR?

Fix pom.xml and relocations. Also removes annotations dependencies and include a integration test to prevent future breakage.

### Are these changes tested?

Yes. A new integration test check the jar content

### Are there any user-facing changes?

Yes. The published pom.xml file on Maven will be cleaned of any dependency
* GitHub Issue: apache#40878

Authored-by: Laurent Goujon <[email protected]>
Signed-off-by: David Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants