Skip to content

fix(bqjdbc): update shading to be more targeted#13232

Open
logachev wants to merge 3 commits into
mainfrom
kirl/less_aggressive_shading
Open

fix(bqjdbc): update shading to be more targeted#13232
logachev wants to merge 3 commits into
mainfrom
kirl/less_aggressive_shading

Conversation

@logachev
Copy link
Copy Markdown
Contributor

  • Fixed shading warnings
  • Adding shaded sources JAR (gemini recommendation for debugging via IDE)
  • More explicit shading for libraries: there was a bug due to aggressive shading of com. It also updated constant strings, such as 'computeMetadata' which is a part of certain URLs which broke GCE auth flow. b/514870009

@logachev logachev requested review from a team as code owners May 20, 2026 05:40
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the maven-shade-plugin to version 3.6.1 and replaces broad package relocation rules with a highly granular list of sub-packages. It also enables source shading and excludes module-info files from the shaded JAR. Feedback indicates that the granular relocation of com.google.api and com.google.cloud is fragile and should be replaced with parent-package relocation and specific exclusions to avoid missing base classes. Furthermore, it is recommended to avoid shading org.slf4j to ensure the driver's logs remain observable by the host application.

Comment thread java-bigquery/google-cloud-bigquery-jdbc/pom.xml Outdated
Comment thread java-bigquery/google-cloud-bigquery-jdbc/pom.xml Outdated
Comment thread java-bigquery/google-cloud-bigquery-jdbc/pom.xml Outdated
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the maven-shade-plugin to version 3.6.1 and refines the shading configuration for the BigQuery JDBC driver. Key changes include enabling the creation of shaded source jars, replacing broad package relocations with a comprehensive list of granular relocations for specific Google, Apache, and third-party libraries, and adding exclusions for module-info files to prevent conflicts in the shaded artifact. I have no feedback to provide as there were no review comments to evaluate.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the maven-shade-plugin to version 3.6.1, enables source shading, and replaces broad relocation patterns with a granular list of sub-packages. It also adds exclusions for module-info files to the shade configuration. Feedback indicates that the manual listing of com.google sub-packages is fragile and potentially incomplete; a more robust approach using a single relocation pattern with specific exclusions for BigQuery packages is recommended to ensure maintainability and avoid classpath conflicts.

Comment thread java-bigquery/google-cloud-bigquery-jdbc/pom.xml
@logachev
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the maven-shade-plugin to version 3.6.1, enables source JAR shading, and replaces broad relocation patterns with a more specific list of subpackages. It also adds exclusions for module-info files. Feedback suggests removing the trailing dot from the com.google. relocation pattern to ensure the base package is included and to maintain consistency with other patterns. Additionally, the reviewer noted that the shift to specific patterns risks missing transitive dependencies like com.github or org.xerial, which could lead to classpath conflicts.

Comment thread java-bigquery/google-cloud-bigquery-jdbc/pom.xml Outdated
Comment thread java-bigquery/google-cloud-bigquery-jdbc/pom.xml
@logachev logachev force-pushed the kirl/less_aggressive_shading branch from 1db93df to a377827 Compare May 21, 2026 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant