Skip to content

feat(kernel): forward identity federation client ID - #910

Open
vuanhphung wants to merge 3 commits into
mainfrom
feature/kernel-identity-federation-client-id
Open

feat(kernel): forward identity federation client ID#910
vuanhphung wants to merge 3 commits into
mainfrom
feature/kernel-identity-federation-client-id

Conversation

@vuanhphung

Copy link
Copy Markdown
Collaborator

Threads identity_federation_client_id from connect() through the kernel auth bridge to the PyO3 Session for PAT, OAuth M2M, and OAuth U2M. This enables SP-wide workload identity federation added in databricks-sql-kernel#248.

Pins KERNEL_REV to the merged kernel commit so kernel E2E builds against the matching API, and updates the public documentation. The published 0.2.0 wheel does not include this API; the dependency floor should be updated after the next kernel wheel release.

Validation: syntax check, whitespace check, and repository commit hooks passed. Unit tests were not run because Poetry is unavailable in the local checkout.

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: 1 Medium

Looks good — the federation client ID is threaded correctly through all three kernel auth paths (M2M/PAT/U2M) with guards consistent with the updated docstrings and the session.py plumbing. One medium concern: the new forwarding behavior has no unit-test coverage in the otherwise-thorough bridge test suite.

Comment thread src/databricks/sql/backend/kernel/auth_bridge.py

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Verdict: 1 Medium

Looks good overall — the identity_federation_client_id threading through the kernel auth bridge, session, and docs is consistent and correct across all three auth paths. One medium concern: the new forwarding behavior has no unit-test coverage despite the existing bridge test suite pinning every other forwarded kwarg.

Comment thread src/databricks/sql/backend/kernel/auth_bridge.py

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No issues identified by the review bot.

Comment thread KERNEL_REV
@@ -1 +1 @@
7ffb30d533c08651ca707b8dd13894c9e01cb68e
eff8950428f4e6cc9975c663ec919f334962f7d0

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.

not sure if it's safe to bump this? is there any SOP?

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.

1 participant