-
Notifications
You must be signed in to change notification settings - Fork 27
[PECOBLR-131][PECOBLR-180] Add token cache for U2M OAuth by using sdk #783
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
Conversation
# Conflicts: # pom.xml # src/main/java/com/databricks/jdbc/api/impl/volume/DBFSVolumeClient.java # src/main/java/com/databricks/jdbc/dbclient/impl/sqlexec/DatabricksSdkClient.java # src/test/java/com/databricks/jdbc/dbclient/impl/sqlexec/DatabricksSdkClientTest.java
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.
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/main/java/com/databricks/jdbc/common/DatabricksJdbcUrlParams.java:121
- [nitpick] Consider using a naming convention consistent with existing parameters (e.g., lowerCamelCase as in "socketTimeout") for consistency in URL parameter keys.
TOKEN_CACHE_PASS_PHRASE("TokenCachePassPhrase", "Pass phrase to use for OAuth U2M Token Cache")
src/main/java/com/databricks/jdbc/api/impl/DatabricksConnectionContext.java
Outdated
Show resolved
Hide resolved
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.
Pull Request Overview
This PR implements token caching for OAuth U2M by integrating support for both a no-operation token cache and an encrypted file token cache, and updates connection context parsing and URL parameters accordingly. Key changes include:
- Adding new tests for token cache configuration and encryption.
- Introducing new helper methods and enum parameters for token cache management in ClientConfigurator and DatabricksJdbcUrlParams.
- Implementing two token cache variants (NoOp and EncryptedFile) with corresponding file permission and encryption logic.
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/databricks/jdbc/dbclient/impl/common/ClientConfiguratorTest.java | Added tests validating U2M configuration with/without token cache. |
| src/test/java/com/databricks/jdbc/auth/NoOpTokenCacheTest.java | Introduced tests for the NoOpTokenCache behavior. |
| src/test/java/com/databricks/jdbc/auth/EncryptedFileTokenCacheTest.java | Added tests for encrypted file token cache including error cases and permission checks. |
| src/test/java/com/databricks/jdbc/api/impl/DatabricksConnectionContextTest.java | Extended tests to verify token cache settings via JDBC URL and properties. |
| src/main/java/com/databricks/jdbc/dbclient/impl/common/ClientConfigurator.java | Added token cache path generation and token cache initialization logic. |
| src/main/java/com/databricks/jdbc/common/DatabricksJdbcUrlParams.java | Added new URL parameters for token cache configuration. |
| src/main/java/com/databricks/jdbc/auth/NoOpTokenCache.java | Implemented a no-operation token cache for disabled caching. |
| src/main/java/com/databricks/jdbc/auth/EncryptedFileTokenCache.java | Implemented encrypted file based token cache with encryption and decryption logic. |
| src/main/java/com/databricks/jdbc/api/internal/IDatabricksConnectionContext.java and src/main/java/com/databricks/jdbc/api/impl/DatabricksConnectionContext.java | Extended connection context interface and implementation with token cache settings. |
Files not reviewed (1)
- pom.xml: Language not supported
src/main/java/com/databricks/jdbc/auth/EncryptedFileTokenCache.java
Outdated
Show resolved
Hide resolved
src/main/java/com/databricks/jdbc/auth/EncryptedFileTokenCache.java
Outdated
Show resolved
Hide resolved
src/test/java/com/databricks/jdbc/dbclient/impl/common/ClientConfiguratorTest.java
Outdated
Show resolved
Hide resolved
src/main/java/com/databricks/jdbc/dbclient/impl/common/ClientConfigurator.java
Outdated
Show resolved
Hide resolved
src/main/java/com/databricks/jdbc/dbclient/impl/common/ClientConfigurator.java
Outdated
Show resolved
Hide resolved
…token-cache # Conflicts: # pom.xml # src/main/java/com/databricks/jdbc/api/internal/IDatabricksConnectionContext.java
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.
Pull Request Overview
This PR adds support for token caching in the OAuth U2M flow by introducing new configuration parameters and implementing both an encrypted file cache and a no-operation cache fallback. Key changes include updates to the client configurator to integrate token caching, new tests covering both encrypted and no-op caching implementations, and corresponding adjustments in the JDBC URL parameters and connection context.
Reviewed Changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/test/java/com/databricks/jdbc/dbclient/impl/common/ClientConfiguratorTest.java | Updates tests to verify token cache functionality; however, expected scopes in assertions remain in legacy array syntax. |
| src/test/java/com/databricks/jdbc/auth/NoOpTokenCacheTest.java | Added tests to validate no-operation token cache behavior. |
| src/test/java/com/databricks/jdbc/auth/EncryptedFileTokenCacheTest.java | Added tests for encrypted token cache persistence and error handling. |
| src/test/java/com/databricks/jdbc/api/impl/DatabricksConnectionContextTest.java | Extended tests to cover new token cache configuration parameters. |
| src/main/java/com/databricks/jdbc/dbclient/impl/common/ClientConfigurator.java | Integrated token cache support into the client setup logic. |
| src/main/java/com/databricks/jdbc/common/DatabricksJdbcUrlParams.java | Introduced new URL parameters for token caching. |
| src/main/java/com/databricks/jdbc/auth/NoOpTokenCache.java | New no-op cache implementation for disabled token caching. |
| src/main/java/com/databricks/jdbc/auth/EncryptedFileTokenCache.java | Implemented encrypted token cache with file IO and encryption. |
| src/main/java/com/databricks/jdbc/api/internal/IDatabricksConnectionContext.java | Extended interface to include token cache methods. |
| src/main/java/com/databricks/jdbc/api/impl/DatabricksConnectionContext.java | Updated connection context to support token cache configuration. |
| NEXT_CHANGELOG.md | Documented new token caching feature in the changelog. |
Files not reviewed (1)
- pom.xml: Language not supported
Description
Depends on databricks/databricks-sdk-java#429
This PR implements token caching for OAuth U2M by integrating support for both a no-operation token cache and an encrypted file token cache, and updates connection context parsing and URL parameters accordingly. Key changes include:
Testing
Manual + unit tests across jdbc and sdk
Additional Notes to the Reviewer