Skip to content

[PECOBLR-762] Make U2M work independent to that of java sdk#954

Merged
samikshya-db merged 8 commits into
databricks:mainfrom
samikshya-db:samikshya-chand_data/u2m
Sep 4, 2025
Merged

[PECOBLR-762] Make U2M work independent to that of java sdk#954
samikshya-db merged 8 commits into
databricks:mainfrom
samikshya-db:samikshya-chand_data/u2m

Conversation

@samikshya-db
Copy link
Copy Markdown
Collaborator

@samikshya-db samikshya-db commented Aug 23, 2025

Description

  • We have had issues with making Azure U2M work in the long term, this PR is to fix these issues in the next release.
  • In this PR we have replicated the simple U2M flow added in our other OSS drivers.

Testing

  • Added unit tests to maintain coverage.
  • Tested manually using Auth_Flow=2 with adb host.
  • Note that one of our customer also tested this out and says it is working for them too.

Additional Notes to the Reviewer

@samikshya-db samikshya-db changed the title Make U2M work independent to that of java sdk [PECOBLR-762] Make U2M work independent to that of java sdk Aug 28, 2025
@samikshya-db samikshya-db marked this pull request as ready for review August 28, 2025 17:20
}

@Override
public String authType() {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit; move this to static constant.

}

if (tokenResponse.has("expires_in")) {
int expiresIn = tokenResponse.get("expires_in").asInt();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually time fields are casted in long and not int. It is within contracts?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that a mature sdk like Okta also has it in int (Link)

I guess we should be ok with this for now.

callbackServer.start(callbackPort);
try {
// Give the server a moment to be fully ready
Thread.sleep(200);
Copy link
Copy Markdown
Collaborator

@nikhilsuri-db nikhilsuri-db Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this 200ms number coming from?

@nikhilsuri-db
Copy link
Copy Markdown
Collaborator

LGTM overall but AzureExternalBrowserProvider seems to be very heavy class. Should we try to move dependent methods to a helper class or a util class?

@samikshya-db
Copy link
Copy Markdown
Collaborator Author

[C1] Thanks for reviewing, @nikhilsuri-db.
As mentioned on the PR description, this is just to unblock certain customers from using Azure U2M which is not currently working for them.

@nikhilsuri-db nikhilsuri-db self-requested a review September 4, 2025 07:49
@samikshya-db samikshya-db enabled auto-merge (squash) September 4, 2025 18:27
@samikshya-db samikshya-db merged commit f10f15e into databricks:main Sep 4, 2025
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants