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

Question: should google-cloud-spanner-jdbc use the Libraries BOM? #252

Closed
suztomo opened this issue Nov 3, 2020 · 4 comments · Fixed by #258
Closed

Question: should google-cloud-spanner-jdbc use the Libraries BOM? #252

suztomo opened this issue Nov 3, 2020 · 4 comments · Fixed by #258
Assignees
Labels
api: spanner Issues related to the googleapis/java-spanner-jdbc API. triage me I really want to be triaged.

Comments

@suztomo
Copy link
Member

suztomo commented Nov 3, 2020

While troubleshooting #251 (comment), I found the pom.xml of this repository imports the Libraries BOM. (Since when? The PR #133 in May 2020 added the Libraries BOM in this project's pom.xml through dependencyManagement section's import. It seems that the PR intended to make it easy to specify the versions of multiple libraries.)

However, the Libraries BOM includes this repository's library (com.google.cloud:google-cloud-spanner-jdbc) through google-cloud-bom. It's a circular dependency. This circular dependency makes dependency troubleshooting difficult in #251 .

Shall we use the shared dependency BOM only and remove the libraries BOM from the dependency management section?

Here is removal of the library versions from pom.xml: https://github.com/googleapis/java-spanner-jdbc/pull/133/files#diff-9c5fb3d1b7e3b0f54bc5c4182965c4fe1f9023d449017cece3005d3f90e8e4d8R59

  • grpc.version
  • api-client.version
  • google.core.version
  • gax.version
  • http-client-bom.version
  • guava.version
  • google.common-protos.version
  • protobuf.version
  • google.api-common.version
  • spanner.version
  • google.auth.version
@product-auto-label product-auto-label bot added the api: spanner Issues related to the googleapis/java-spanner-jdbc API. label Nov 3, 2020
@suztomo
Copy link
Member Author

suztomo commented Nov 3, 2020

@stephaniewang526 @elharo @chingor13 What do you think?

@olavloite
Copy link
Collaborator

@suztomo Just to supply some more context on #133: The JDBC driver repository used to contain two main components:

  • A generic Connection API for Cloud Spanner that was not exposed publicly.
  • The JDBC driver that was a thin layer on top of the generic Connection API.

We wanted to expose the generic Connection API publicly for other purposes than only the JDBC driver, so that part of the code was moved to the java-spanner repository in an earlier PR. #133 removed the code of the Connection API from the JDBC driver repository and changed the code to use the Connection API from the java-spanner repository instead.

So from the perspective of the intention behind #133, it's perfectly reasonable to change how the dependencies are included in the JDBC driver as long as it is able to use the needed components from the java-spanner repository, if that fixes the dependency problems we are currently experiencing.

@yoshi-automation yoshi-automation added the triage me I really want to be triaged. label Nov 4, 2020
@suztomo
Copy link
Member Author

suztomo commented Nov 4, 2020

@olavloite Thank you for explanation.

@suztomo
Copy link
Member Author

suztomo commented Nov 6, 2020

Simply removing the libraries-bom generates the following errors:

[ERROR] The build could not read 1 project -> [Help 1]
[ERROR]   
[ERROR]   The project com.google.cloud:google-cloud-spanner-jdbc:1.17.3-SNAPSHOT (/Users/suztomo/java-spanner-jdbc/pom.xml) has 6 errors
[ERROR]     'dependencies.dependency.version' for com.google.cloud:google-cloud-spanner:jar is missing. @ line 93, column 17
[ERROR]     'dependencies.dependency.version' for com.google.api.grpc:proto-google-cloud-spanner-v1:jar is missing. @ line 137, column 17
[ERROR]     'dependencies.dependency.version' for com.google.api.grpc:grpc-google-cloud-spanner-v1:jar is missing. @ line 153, column 17
[ERROR]     'dependencies.dependency.version' for com.google.cloud:google-cloud-spanner:test-jar is missing. @ line 147, column 17
[ERROR]     'dependencies.dependency.version' for com.google.api.grpc:grpc-google-cloud-spanner-admin-instance-v1:jar is missing. @ line 158, column 17
[ERROR]     'dependencies.dependency.version' for com.google.api.grpc:grpc-google-cloud-spanner-admin-database-v1:jar is missing. @ line 163, column 17
[ERROR] 

Continuing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the googleapis/java-spanner-jdbc API. triage me I really want to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants