Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Generalise Backend Layer#604

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
varun-edachali-dbx merged 35 commits intomainfrombackend-refactors
Jul 15, 2025
Merged

Generalise Backend Layer#604

varun-edachali-dbx merged 35 commits intomainfrombackend-refactors
Jul 15, 2025

Conversation

@varun-edachali-dbx
Copy link
Contributor

@varun-edachali-dbxvarun-edachali-dbx commentedJun 18, 2025•
edited
Loading

What type of PR is this?

  • Refactor

Description

This PR refactors the codebase to aid the eventual introduction of a new backend client. It is composed of the following three PRs:

  • Separate theSession functionality from theConnection class to further abstract backend implementation details from the connection class:Separate Session related functionality from Connection class #571
  • Introduce a general backend interface (DatabricksClient):Introduce Backend Interface (DatabricksClient) #573
    • Introduce theDatabricksClient interface and make the existingThriftBackend implement it asThriftDatabricksClient.
    • IntroduceSessionId andCommandId as consistent adapters for backend layers to represent sessions and commands instead of relying on Thrift specific types.
  • Introduce a generalResultSet interface:Implement ResultSet Abstraction (backend interfaces for fetch phase) #574
    • Create aResultSet interface and make the existing ResultSet interface implement it asThriftResultSet.
    • Make the concrete Thrift backend return aThriftResultSet instead ofExecuteResponse from execution relevant commands.
    • Generalise the return types of methods in the backend interface to prevent alignment with Thrift (introduceCommandState to prevent using Thrift's status types).

How is this tested?

  • Unit tests
  • E2E Tests
  • Manually
  • N/A

Related Tickets & Documents

Design Doc
PECOBLR-569

varun-edachali-dbxand others added4 commitsJune 18, 2025 03:47
* decouple session class from existing Connectionensure maintenance of current APIs of Connection while delegatingresponsibilitySigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add open property to Connection to ensure maintenance of existing APISigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* update unit tests to address ThriftBackend through session instead of through ConnectionSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* chore: move session specific tests from test_client to test_sessionSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black)as in CONTRIBUTING.mdSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* use connection open property instead of long chain through sessionSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* trigger integration workflowSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix: ensure open attribute of Connection never failsin case the openSession takes long, the initialisation of the sessionwill not complete immediately. This could make the session attributeinaccessible. If the Connection is deleted in this time, the open()check will throw because the session attribute does not exist. Thus, wedefault to the Connection being closed in this case. This was not anissue before because open was a direct attribute of the Connectionclass. Caught in the integration tests.Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix: de-complicate earlier connection open logicearlier, one of the integration tests was failing because 'session wasnot an attribute of Connection'. This is likely tied to a localconfiguration issue related to unittest that was causing an error in thetest suite itself. The tests are now passing without checking for thesession attribute.c676f9bSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* Revert "fix: de-complicate earlier connection open logic"This reverts commitd6b1b19.Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* [empty commit] attempt to trigger ci e2e workflowSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* Update CODEOWNERS (#562)new codeownersSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* Enhance Cursor close handling and context manager exception management to prevent server side resource leaks (#554)* Enhance Cursor close handling and context manager exception management* tests* fmt* Fix Cursor.close() to properly handle CursorAlreadyClosedError* Remove specific test message from Cursor.close() error handling* Improve error handling in connection and cursor context managers to ensure proper closure during exceptions, including KeyboardInterrupt. Add tests for nested cursor management and verify operation closure on server-side errors.* add* addSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* PECOBLR-86 improve logging on python driver (#556)* PECOBLR-86 Improve logging for debug levelSigned-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>* PECOBLR-86 Improve logging for debug levelSigned-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>* fixed formatSigned-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>* used lazy loggingSigned-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>* changed debug to error logsSigned-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>* used lazy loggingSigned-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>---------Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* Revert "Merge remote-tracking branch 'upstream/sea-migration' into decouple-session"This reverts commitdbb2ec5, reversingchanges made to7192f11.Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* Reapply "Merge remote-tracking branch 'upstream/sea-migration' into decouple-session"This reverts commitbdb8381.Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix: separate session opening logic from instantiationensures correctness of self.session.open call in ConnectionSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix: use is_open attribute to denote session availabilitySigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix: access thrift backend through sessionSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* chore: use get_handle() instead of private session attribute in clientSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix: remove accidentally removed assertionsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>---------Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>Co-authored-by: Jothi Prakash <jothi.prakash@databricks.com>Co-authored-by: Madhav Sainanee <madhav.sainanee@databricks.com>Co-authored-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>
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, instantiationslarge merge artifactsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black) + remove excess log (merge artifact)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix typingSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove un-necessary checkSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove un-necessary replace callSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* introduce __str__ methods for CommandId and SessionIdSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* docstrings for DatabricksClient interfaceSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* stronger typing of Cursor and ExecuteResponseSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove utility functions from backend interface, fix circular importSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* rename info to propertiesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* newline for cleanlinessSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix circular importSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* to_hex_id -> get_hex_idSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* better comment on protocol version getterSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* move guid to hex id to new utils moduleSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* move staging allowed local path to connection propsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add strong return type for execute_commandSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* skip auth, error handling in databricksclient interfaceSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* chore: docstring + line widthSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* get_id -> get_guidSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* chore: docstringSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix: to_hex_id -> to_hex_guidSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>---------Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
…574)* ensure backend client returns a ResultSet type in backend testsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* newline for cleanlinessSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* fix circular importSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* to_hex_id -> get_hex_idSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* better comment on protocol version getterSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* formatting (black)Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* stricter typing for cursorSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* correct typingSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* correct tests and merge artifactsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove accidentally modified workflow filesremnants of old mergeSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* chore: remove accidentally modified workflow filesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add back accidentally removed docstringsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* clean up docstringsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* log hexSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove unnecessary _replace callSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add __str__ for CommandIdSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* take TOpenSessionResp in get_protocol_version to maintain existing interfaceSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* active_op_handle -> active_mmand_idSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* ensure None returned for close_commandSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* account for ResultSet return in new pydocsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* pydoc for typesSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* move common state to ResultSet aprentSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* stronger typing in resultSet behaviourSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove redundant patch in testSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add has_been_closed_server_side assertionSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* remove redundancies in testsSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* more robust close checkSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* use normalised state in e2e testSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* simplify corrected testSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* add line gaps after multi-line pydocs for consistencySigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>* use normalised CommandState type in ExecuteResponseSigned-off-by: varun-edachali-dbx <varun.edachali@databricks.com>---------Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Copy link
Contributor

@jayantsing-dbjayantsing-db left a comment

Choose a reason for hiding this comment

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

Great work, i think the implementation of sql connector makes more sense now and much more open to extension. Some comments inline.

Thanks very much@jprakash-db for the review.

Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Copy link
Contributor

@vikrantpuppalavikrantpuppala left a comment

Choose a reason for hiding this comment

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

telemetry changes lgtm

varun-edachali-dbx reacted with thumbs up emoji
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com>
@varun-edachali-dbxvarun-edachali-dbx merged commitba1eab3 intomainJul 15, 2025
23 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@jayantsing-dbjayantsing-dbjayantsing-db approved these changes

@vikrantpuppalavikrantpuppalavikrantpuppala approved these changes

@jprakash-dbjprakash-dbAwaiting requested review from jprakash-db

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@varun-edachali-dbx@vikrantpuppala@jayantsing-db@jprakash-db
[8]ページ先頭
©2009-2025 Movatter.jp