-
Notifications
You must be signed in to change notification settings - Fork 26
Fix regression in sdk (changes in token federation) #1052
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
Merged
gopalldb
merged 3 commits into
databricks:main
from
samikshya-db:samikshya-chand_data/ohNoAnotherRegression
Oct 29, 2025
Merged
Fix regression in sdk (changes in token federation) #1052
gopalldb
merged 3 commits into
databricks:main
from
samikshya-db:samikshya-chand_data/ohNoAnotherRegression
Oct 29, 2025
+25
−10
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gopalldb
reviewed
Oct 29, 2025
| assertEquals("https://sample-host.18.azuredatabricks.net", config.getHost()); | ||
| assertEquals("test-client", config.getClientId()); | ||
| assertEquals("custom-oauth-m2m", provider.authType()); | ||
| assertEquals(DatabricksJdbcConstants.M2M_AUTH_TYPE, config.getAuthType()); |
Collaborator
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.
no other tests needed?
gopalldb
reviewed
Oct 29, 2025
| // because URLs are generated inside SDK | ||
| if (DriverUtil.isRunningAgainstFake()) { | ||
| return this.credentialsProvider.configure(databricksConfig); | ||
| } |
Collaborator
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.
is configure always called once?
gopalldb
approved these changes
Oct 29, 2025
gopalldb
added a commit
that referenced
this pull request
Oct 29, 2025
- The issue : - An [E2E test](https://docs.google.com/document/d/1McX4IgD-ZBTtiNXNUrEemsjbVj3oeoQYDwegjjln6UA/edit?ouid=113104269373381935368&tab=t.0#heading=h.k832j8h2svg) conducted with JDBC, with additional SDK-level logging, confirmed that the SDK was refreshing tokens on every call—even when successive calls occurred within seconds. This behavior occurred despite Databricks M2M tokens being valid for 59 minutes. As a result, the token endpoint was hit excessively, eventually triggering global rate limits and throttling for the IP. - Each [getToken call in the SDK](https://github.com/databricks/databricks-jdbc/blob/a17b84c1a0418094a8434f56246c764fa235d19b/src/main/java/com/databricks/jdbc/dbclient/impl/thrift/DatabricksHttpTTransport.java#L166) fetched a new token from the server because the SDK did not cache tokens correctly. This was traced to a regression in the SDK’s token management logic. - This PR ensures that the SDK is configured once in the constructor, preventing repeated token endpoint calls. We also plan to perform a broader SDK code audit to identify and address any similar issues going forward. - Tested manually using M2M flow : The token retrieval is now performed only once when M2M creds are used. - Unit tests - Internal doc : https://docs.google.com/document/d/1McX4IgD-ZBTtiNXNUrEemsjbVj3oeoQYDwegjjln6UA/edit?tab=t.g07tag19b223 --------- Co-authored-by: Gopal Lal <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Testing
Additional Notes to the Reviewer