Introduce Backend Interface (DatabricksClient) #573

varun-edachali-dbx · 2025-05-26T09:10:46Z

What type of PR is this?

Refactor

Description

Introduce the DatabricksClient interface and make the existing thrift backend implement it. This allows the cursor to not be aware of the type of backend instantiated. Currently, we have to include some assertions in the ResultSet to ensure we have a ThriftDatabricksClient type, because the fetch-phase abstractions have not been implemented yet.
Introduce SessionId and CommandId interfaces to create a consistent adapter to be used to represent sessions and commands instead of relying on Thrift (or eventually, SEA) specific types.

How is this tested?

Unit tests
- some of the existing unit tests were slightly altered to account for the introduction of the interface. No unit tests were removed or introduced.
E2E Tests
Manually
N/A

Related Tickets & Documents

https://docs.google.com/document/d/1Y-eXLhNqqhrMVGnOlG8sdFrCxBTN1GdQvuKG4IfHmo0/edit?usp=sharing
https://databricks.atlassian.net/browse/PECOBLR-440?atlOrigin=eyJpIjoiMTgzNGNiMDVkMGQ3NDM2Njg5OTRhZWQ1MGQ4Mjg1OWIiLCJwIjoiaiJ9

github-actions · 2025-05-26T09:14:37Z

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

github-actions · 2025-05-26T10:33:57Z

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

github-actions · 2025-05-26T12:53:36Z

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Signed-off-by: varun-edachali-dbx <[email protected]>

src/databricks/sql/backend/databricks_client.py

Signed-off-by: varun-edachali-dbx <[email protected]>

jayantsing-db

Thanks for changes and most importantly changes to tests. Some minor nit comments, rest LGTM

src/databricks/sql/backend/databricks_client.py

src/databricks/sql/backend/utils/guid_utils.py

src/databricks/sql/client.py

tests/unit/test_client.py

tests/unit/test_parameters.py

tests/unit/test_thrift_backend.py

jayantsing-db · 2025-05-30T12:45:11Z

please make a note of the failing integration test in commit message when pushing if it is unrelated to these changes.

Signed-off-by: varun-edachali-dbx <[email protected]>

samikshya-db · 2025-05-30T16:33:02Z

In the future, let's try to raise smaller size PRs typically within 700 lines of code changes including tests. This will help you get more reviews and more importantly more thorough reviews. Thanks.

jayantsing-db · 2025-05-30T16:36:06Z

In the future, let's try to raise smaller size PRs typically within 700 lines of code changes including tests. This will help you get more reviews and more importantly more thorough reviews. Thanks.

Hey @samikshya-db, it's tricky to further scope this down. Otherwise, it was adding too much overhead.

samikshya-db · 2025-05-30T16:42:40Z

@jayantsing-db I understand this would be harder for the initial set of SEA PRs. Even then, it is good to keep this in mind and try to break it down. Happy to brainstorm on this too.

jayantsing-db · 2025-05-30T16:46:56Z

Even then, it is good to keep this in mind and try to break it down. Happy to brainstorm on this too.

yes

I understand this would be harder for the initial set of SEA PRs

heads up, this and couple more are refactoring PRs to prepare for SEA changes. refactoring PRs are usually high in LOC because we have to make sure current set of tests do not break and so the related changes have to go in together. Even then @varun-edachali-dbx first session re-factoring PR is ~500

for SEA PR, LOC will be low as expected per PR.

NOTE: the `test_complex_types` e2e test was not working at the time of this merge. The test must be triggered when the test is back up and running as intended. * remove excess logs, assertions, instantiations large merge artifacts Signed-off-by: varun-edachali-dbx <[email protected]> * formatting (black) + remove excess log (merge artifact) Signed-off-by: varun-edachali-dbx <[email protected]> * fix typing Signed-off-by: varun-edachali-dbx <[email protected]> * remove un-necessary check Signed-off-by: varun-edachali-dbx <[email protected]> * remove un-necessary replace call Signed-off-by: varun-edachali-dbx <[email protected]> * introduce __str__ methods for CommandId and SessionId Signed-off-by: varun-edachali-dbx <[email protected]> * docstrings for DatabricksClient interface Signed-off-by: varun-edachali-dbx <[email protected]> * stronger typing of Cursor and ExecuteResponse Signed-off-by: varun-edachali-dbx <[email protected]> * remove utility functions from backend interface, fix circular import Signed-off-by: varun-edachali-dbx <[email protected]> * rename info to properties Signed-off-by: varun-edachali-dbx <[email protected]> * newline for cleanliness Signed-off-by: varun-edachali-dbx <[email protected]> * fix circular import Signed-off-by: varun-edachali-dbx <[email protected]> * formatting (black) Signed-off-by: varun-edachali-dbx <[email protected]> * to_hex_id -> get_hex_id Signed-off-by: varun-edachali-dbx <[email protected]> * better comment on protocol version getter Signed-off-by: varun-edachali-dbx <[email protected]> * formatting (black) Signed-off-by: varun-edachali-dbx <[email protected]> * move guid to hex id to new utils module Signed-off-by: varun-edachali-dbx <[email protected]> * formatting (black) Signed-off-by: varun-edachali-dbx <[email protected]> * move staging allowed local path to connection props Signed-off-by: varun-edachali-dbx <[email protected]> * add strong return type for execute_command Signed-off-by: varun-edachali-dbx <[email protected]> * skip auth, error handling in databricksclient interface Signed-off-by: varun-edachali-dbx <[email protected]> * chore: docstring + line width Signed-off-by: varun-edachali-dbx <[email protected]> * get_id -> get_guid Signed-off-by: varun-edachali-dbx <[email protected]> * chore: docstring Signed-off-by: varun-edachali-dbx <[email protected]> * fix: to_hex_id -> to_hex_guid Signed-off-by: varun-edachali-dbx <[email protected]> --------- Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx requested review from deeksha-db, samikshya-db, jprakash-db, jackyhu-db, madhav-db, gopalldb, jayantsing-db, vikrantpuppala and shivam2680 as code owners May 26, 2025 09:10

varun-edachali-dbx had a problem deploying to azure-prod May 26, 2025 09:12 — with GitHub Actions Failure

varun-edachali-dbx force-pushed the backend-interface branch from a251d75 to 1df7f02 Compare May 26, 2025 09:19

varun-edachali-dbx had a problem deploying to azure-prod May 26, 2025 09:19 — with GitHub Actions Failure

varun-edachali-dbx had a problem deploying to azure-prod May 26, 2025 10:28 — with GitHub Actions Failure

varun-edachali-dbx had a problem deploying to azure-prod May 26, 2025 10:33 — with GitHub Actions Failure

varun-edachali-dbx force-pushed the backend-interface branch from 7da3b63 to 49af2ea Compare May 26, 2025 10:34

varun-edachali-dbx had a problem deploying to azure-prod May 26, 2025 10:34 — with GitHub Actions Failure

varun-edachali-dbx had a problem deploying to azure-prod May 26, 2025 10:35 — with GitHub Actions Failure

varun-edachali-dbx marked this pull request as draft May 26, 2025 10:42

varun-edachali-dbx had a problem deploying to azure-prod May 26, 2025 12:53 — with GitHub Actions Failure

varun-edachali-dbx force-pushed the backend-interface branch from 207ae8c to 169716c Compare May 26, 2025 12:54

varun-edachali-dbx had a problem deploying to azure-prod May 26, 2025 12:54 — with GitHub Actions Failure

varun-edachali-dbx had a problem deploying to azure-prod May 26, 2025 14:57 — with GitHub Actions Failure

varun-edachali-dbx had a problem deploying to azure-prod May 29, 2025 04:22 — with GitHub Actions Failure

formatting (black)

1ec8c45

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx had a problem deploying to azure-prod May 29, 2025 04:25 — with GitHub Actions Failure

jayantsing-db reviewed May 29, 2025

View reviewed changes

src/databricks/sql/backend/databricks_client.py Outdated Show resolved Hide resolved

src/databricks/sql/backend/databricks_client.py Outdated Show resolved Hide resolved

jayantsing-db reviewed May 29, 2025

View reviewed changes

src/databricks/sql/backend/databricks_client.py Outdated Show resolved Hide resolved

move guid to hex id to new utils module

bc5e716

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx had a problem deploying to azure-prod May 29, 2025 15:57 — with GitHub Actions Failure

formatting (black)

8b6ba56

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx had a problem deploying to azure-prod May 29, 2025 15:59 — with GitHub Actions Failure

move staging allowed local path to connection props

c0b989c

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx had a problem deploying to azure-prod May 29, 2025 16:30 — with GitHub Actions Failure

varun-edachali-dbx requested a review from jayantsing-db May 30, 2025 06:15

jayantsing-db approved these changes May 30, 2025

View reviewed changes

varun-edachali-dbx added 5 commits May 30, 2025 16:02

add strong return type for execute_command

ece7616

Signed-off-by: varun-edachali-dbx <[email protected]>

skip auth, error handling in databricksclient interface

765bbd7

Signed-off-by: varun-edachali-dbx <[email protected]>

chore: docstring + line width

54077f7

Signed-off-by: varun-edachali-dbx <[email protected]>

get_id -> get_guid

91357f4

Signed-off-by: varun-edachali-dbx <[email protected]>

chore: docstring

26c017d

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx had a problem deploying to azure-prod May 30, 2025 16:12 — with GitHub Actions Failure

fix: to_hex_id -> to_hex_guid

8f085d5

Signed-off-by: varun-edachali-dbx <[email protected]>

varun-edachali-dbx had a problem deploying to azure-prod May 30, 2025 16:16 — with GitHub Actions Failure

varun-edachali-dbx merged commit 400a8bd into sea-migration May 30, 2025
22 of 23 checks passed

varun-edachali-dbx mentioned this pull request Jun 18, 2025

Generalise Backend Layer #604

Open

5 tasks

Introduce Backend Interface (DatabricksClient) #573

Introduce Backend Interface (DatabricksClient) #573

Uh oh!

Conversation

varun-edachali-dbx commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What type of PR is this?

Description

How is this tested?

Related Tickets & Documents

Uh oh!

github-actions bot commented May 26, 2025

Uh oh!

github-actions bot commented May 26, 2025

Uh oh!

github-actions bot commented May 26, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jayantsing-db left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jayantsing-db commented May 30, 2025

Uh oh!

samikshya-db commented May 30, 2025

Uh oh!

jayantsing-db commented May 30, 2025

Uh oh!

samikshya-db commented May 30, 2025

Uh oh!

jayantsing-db commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

varun-edachali-dbx commented May 26, 2025 •

edited

Loading

jayantsing-db commented May 30, 2025 •

edited

Loading