-
Notifications
You must be signed in to change notification settings - Fork 123
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
feat: support CredentialsProvider in Connection API #1869
Conversation
Adds suppport for setting a CredentialsProvider instead of a credentialsUrl in a connection string. The CredentialsProvider reference must be a class name to a public class with a public no-arg constructor. This option is available in the Connection API, which means that any client that uses that API can directly benefit from it (this effectively means the JDBC driver). Fixes b/231174409
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with a couple of small points/questions.
google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java
Outdated
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/SpannerPool.java
Show resolved
Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionOptions.java
Show resolved
Hide resolved
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
Merging manually, as automerge failed on this PR (even though all checks have passed). |
testConnectWithCredentialsAndOAuthToken test was failing with following error - JdbcDriverTest.testConnectWithCredentialsAndOAuthToken:135 expected to contain: Cannot specify both credentials and an OAuth token but was : INVALID_ARGUMENT: Specify only one of credentialsUrl, encodedCredentials, credentialsProvider and OAuth token. Changing assertThat to account for this change. The exception message was changed as part of googleapis/java-spanner#1869.
testConnectWithCredentialsAndOAuthToken test was failing with following error - JdbcDriverTest.testConnectWithCredentialsAndOAuthToken:135 expected to contain: Cannot specify both credentials and an OAuth token but was : INVALID_ARGUMENT: Specify only one of credentialsUrl, encodedCredentials, credentialsProvider and OAuth token. Changing assertThat to account for this change. The exception message was changed as part of googleapis/java-spanner#1869.
Adds support for setting a
CredentialsProvider
instead of acredentialsUrl in a connection string. The
CredentialsProvider
referencemust be a class name to a public class with a public no-arg constructor.
This option is available in the Connection API, which means that any
client that uses that API can directly benefit from it (this effectively
means the JDBC driver).
The option will only be available in the JDBC driver once the dependency on the client library in the JDBC driver has been updated to a version of the Java client library that contains this change.
Fixes b/231174409