Refactor SessionCredentials#470
Conversation
| * | ||
| * <p>Implements the refresh_token OAuth grant type with optional token caching. | ||
| */ | ||
| public class SessionCredentialsTokenSource extends RefreshableTokenSource { |
There was a problem hiding this comment.
Can we AutoValue this and use an auto value builder?
There was a problem hiding this comment.
After some reading up, it seems like AutoValue does not support constructors that call parent constructors with arguments: https://stackoverflow.com/questions/45069608/autovalue-how-to-call-super. In this case, we need super(token) in the constructor, so I have opted for removing the builder class and just have the two optional fields as Optional.
parthban-db
left a comment
There was a problem hiding this comment.
Where is the sessionCredentials used now? I am seeing all its usages are changed to sessionCredentialsTokenSource. Maybe I am missing something.
So SessionCredentials is not used internally as a CredentialsProvider, but based on the examples from https://github.com/databricks/databricks-sdk-java/blob/main/examples/spring-boot-oauth-u2m-demo/src/main/java/com/databricks/sdk/RootController.java it seems like the intention is that a user can get a SessionCredentials and set it as the CredentialsProvider to use. |
parthban-db
left a comment
There was a problem hiding this comment.
LGTM modulo Renaud comments.
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
| * | ||
| * <p>Implements the refresh_token OAuth grant type with optional token caching. | ||
| */ | ||
| public class SessionCredentialsTokenSource extends RefreshableTokenSource { |
What changes are proposed in this pull request?
This PR refactors the
SessionCredentialsclass to achieve clearer separation of concerns by splitting its responsibilities into two distinct components: aCredentialsProviderimplementation and a dedicatedTokenSourceimplementation. This refactoring is necessary for introducing token cache wrappers in future PRs.Current Implementation
SessionCredentialsboth implementsCredentialsProviderand extendsRefreshableTokenSource.Proposed Changes
SessionCredentialsnow implements onlyCredentialsProviderand delegates all token operations to a newSessionCredentialsTokenSourceclass.SessionCredentialsTokenSourceextendsRefreshableTokenSourceand is solely responsible for token refresh logic.Benefits
SessionCredentialsremains a publicCredentialsProvideras intended with no breaking changes to the public API.How is this tested?
Existing unit tests.
NO_CHANGELOG=true