Skip to content

Separate Session related functionality from Connection class #571

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
merged 25 commits into from
May 28, 2025

Conversation

varun-edachali-dbx
Copy link
Collaborator

@varun-edachali-dbx varun-edachali-dbx commented May 26, 2025

What type of PR is this?

  • Refactor

Description

Separate the Session related functionality from the Connection class in order to further abstract the backend implementation details from the connection, to eventually give way to the introduction of SEA.

How is this tested?

  • Unit tests
    • some of the existing unit tests were slightly altered to account for the introduction of the Session class. 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-438?atlOrigin=eyJpIjoiYjBiMjlhYjBjM2RhNDdiOGE4OGU4NjgwYzU1ZmIzNzciLCJwIjoiaiJ9

varun-edachali-dbx and others added 23 commits May 24, 2025 02:25
ensure maintenance of current APIs of Connection while delegating
responsibility

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

Signed-off-by: varun-edachali-dbx <[email protected]>
as in CONTRIBUTING.md

Signed-off-by: varun-edachali-dbx <[email protected]>
Signed-off-by: varun-edachali-dbx <[email protected]>
in case the openSession takes long, the initialisation of the session
will not complete immediately. This could make the session attribute
inaccessible. If the Connection is deleted in this time, the open()
check will throw because the session attribute does not exist. Thus, we
default to the Connection being closed in this case. This was not an
issue before because open was a direct attribute of the Connection
class. Caught in the integration tests.

Signed-off-by: varun-edachali-dbx <[email protected]>
earlier, one of the integration tests was failing because 'session was
not an attribute of Connection'. This is likely tied to a local
configuration issue related to unittest that was causing an error in the
test suite itself. The tests are now passing without checking for the
session attribute.
c676f9b

Signed-off-by: varun-edachali-dbx <[email protected]>
This reverts commit d6b1b19.

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

Signed-off-by: varun-edachali-dbx <[email protected]>
…t 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

* add

Signed-off-by: varun-edachali-dbx <[email protected]>
* PECOBLR-86 Improve logging for debug level

Signed-off-by: Sai Shree Pradhan <[email protected]>

* PECOBLR-86 Improve logging for debug level

Signed-off-by: Sai Shree Pradhan <[email protected]>

* fixed format

Signed-off-by: Sai Shree Pradhan <[email protected]>

* used lazy logging

Signed-off-by: Sai Shree Pradhan <[email protected]>

* changed debug to error logs

Signed-off-by: Sai Shree Pradhan <[email protected]>

* used lazy logging

Signed-off-by: Sai Shree Pradhan <[email protected]>

---------

Signed-off-by: Sai Shree Pradhan <[email protected]>
Signed-off-by: varun-edachali-dbx <[email protected]>
…couple-session"

This reverts commit dbb2ec5, reversing
changes made to 7192f11.

Signed-off-by: varun-edachali-dbx <[email protected]>
…ecouple-session"

This reverts commit bdb8381.

Signed-off-by: varun-edachali-dbx <[email protected]>
ensures correctness of self.session.open call in Connection

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

@jayantsing-db jayantsing-db left a comment

Choose a reason for hiding this comment

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

Hi Varun, thanks for these changes. I have added some minor comments. Please take a look.

Comment on lines +17 to +25
self,
server_hostname: str,
http_path: str,
http_headers: Optional[List[Tuple[str, str]]] = None,
session_configuration: Optional[Dict[str, Any]] = None,
catalog: Optional[str] = None,
schema: Optional[str] = None,
_use_arrow_native_complex_types: Optional[bool] = True,
**kwargs,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit

  • these are identical to connection params. are all of them relevant to a DBSQL session?
  • should be introduce a type/namedtuple like ConnectionParams or a better name. The reason is that now for each added connection param, we would have to modify at two places

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree, that makes sense.

Shouldn't we name it SessionParams though? These parameters will be passed through to the session constructor.

Copy link
Contributor

Choose a reason for hiding this comment

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

I believe Session is an internal abstraction and these params originate from Connection so better to name ConnectionParams. Additionally, i think this change might break a lot of things. Let's do it completely separately (let's log a JIRA ticket for now and take it up later)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Comment on lines +101 to +102
precedence over the serverProtocolVersion defined in the OpenSessionResponse.
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
precedence over the serverProtocolVersion defined in the OpenSessionResponse.
"""
precedence over the serverProtocolVersion defined in the OpenSessionResponse.
"""

i think there is a line gap after a multi-line pydoc. @jprakash-db do we follow any python coding guidelines like for docstring: https://peps.python.org/pep-0257/

Copy link
Contributor

Choose a reason for hiding this comment

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

in databricks we follow this https://databricks.atlassian.net/wiki/spaces/UN/pages/3334538555/Python+Guidelines+go+py but this could be different for OSS repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

We use the black formatter which follows the PEP-257 style.

Copy link
Contributor

Choose a reason for hiding this comment

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

got it. is there a linter? in the CI or do we have to run the linter manually?

Copy link
Contributor

Choose a reason for hiding this comment

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

Currently we have pylint but it needs to be run manually.

Comment on lines +320 to +333
@property
def protocol_version(self):
"""Get the protocol version from the Session object"""
return self.session.protocol_version

@staticmethod
def get_protocol_version(openSessionResp):
"""Delegate to Session class static method"""
return Session.get_protocol_version(openSessionResp)

@property
def open(self) -> bool:
"""Return whether the connection is open by checking if the session is open."""
return self.session.is_open
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should we group together property and staticmethod? @jprakash-db any coding/lint guidelines OSS python driver follows?

Copy link
Contributor

Choose a reason for hiding this comment

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

There are no specific standard in python, because saying something as private etc has no meaning, we can access anything anytime. There are some general standards but nothing concrete

except Exception as e:
logger.error(f"Attempt to close session raised a local exception: {e}")

self.is_open = False
Copy link
Contributor

Choose a reason for hiding this comment

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

when opening session the flag is set at the end which makes sense. for closing session call, should we be eager to unset the flag in the very beginning?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What if the session close fails? Shouldn't the session remain open?

Copy link
Contributor

Choose a reason for hiding this comment

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

if the client code has called on the Session class to close the session, then the client assumes that method will close the session. I think unsetting the flag right away makes more sense then. However, an interesting question is do we use this flag internally in Session class to make unsetting meaningful (i.e., when flag is false, do we give null or throw exception when getting session handle?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently there does not seem to exist such a dependency, but I'm still not clear on this.

If the close() call raises an exception isn't the client expected to retry?

Copy link
Collaborator Author

@varun-edachali-dbx varun-edachali-dbx May 26, 2025

Choose a reason for hiding this comment

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

Copy link

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).

Copy link
Contributor

@jayantsing-db jayantsing-db left a comment

Choose a reason for hiding this comment

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

thanks!

@varun-edachali-dbx varun-edachali-dbx merged commit 5bf5d4c into sea-migration May 28, 2025
23 checks passed
varun-edachali-dbx added a commit that referenced this pull request Jun 18, 2025
* decouple session class from existing Connection

ensure maintenance of current APIs of Connection while delegating
responsibility

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

* add open property to Connection to ensure maintenance of existing API

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

* update unit tests to address ThriftBackend through session instead of through Connection

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

* chore: move session specific tests from test_client to test_session

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

* formatting (black)

as in CONTRIBUTING.md

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

* use connection open property instead of long chain through session

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

* trigger integration workflow

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

* fix: ensure open attribute of Connection never fails

in case the openSession takes long, the initialisation of the session
will not complete immediately. This could make the session attribute
inaccessible. If the Connection is deleted in this time, the open()
check will throw because the session attribute does not exist. Thus, we
default to the Connection being closed in this case. This was not an
issue before because open was a direct attribute of the Connection
class. Caught in the integration tests.

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

* fix: de-complicate earlier connection open logic

earlier, one of the integration tests was failing because 'session was
not an attribute of Connection'. This is likely tied to a local
configuration issue related to unittest that was causing an error in the
test suite itself. The tests are now passing without checking for the
session attribute.
c676f9b

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

* Revert "fix: de-complicate earlier connection open logic"

This reverts commit d6b1b19.

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

* [empty commit] attempt to trigger ci e2e workflow

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

* Update CODEOWNERS (#562)

new codeowners

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

* 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

* add

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

* PECOBLR-86 improve logging on python driver (#556)

* PECOBLR-86 Improve logging for debug level

Signed-off-by: Sai Shree Pradhan <[email protected]>

* PECOBLR-86 Improve logging for debug level

Signed-off-by: Sai Shree Pradhan <[email protected]>

* fixed format

Signed-off-by: Sai Shree Pradhan <[email protected]>

* used lazy logging

Signed-off-by: Sai Shree Pradhan <[email protected]>

* changed debug to error logs

Signed-off-by: Sai Shree Pradhan <[email protected]>

* used lazy logging

Signed-off-by: Sai Shree Pradhan <[email protected]>

---------

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

* Revert "Merge remote-tracking branch 'upstream/sea-migration' into decouple-session"

This reverts commit dbb2ec5, reversing
changes made to 7192f11.

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

* Reapply "Merge remote-tracking branch 'upstream/sea-migration' into decouple-session"

This reverts commit bdb8381.

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

* fix: separate session opening logic from instantiation

ensures correctness of self.session.open call in Connection

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

* fix: use is_open attribute to denote session availability

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

* fix: access thrift backend through session

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

* chore: use get_handle() instead of private session attribute in client

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

* formatting (black)

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

* fix: remove accidentally removed assertions

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

---------

Signed-off-by: varun-edachali-dbx <[email protected]>
Signed-off-by: Sai Shree Pradhan <[email protected]>
Co-authored-by: Jothi Prakash <[email protected]>
Co-authored-by: Madhav Sainanee <[email protected]>
Co-authored-by: Sai Shree Pradhan <[email protected]>
varun-edachali-dbx added a commit that referenced this pull request Jun 18, 2025
* decouple session class from existing Connection

ensure maintenance of current APIs of Connection while delegating
responsibility

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

* add open property to Connection to ensure maintenance of existing API

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

* update unit tests to address ThriftBackend through session instead of through Connection

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

* chore: move session specific tests from test_client to test_session

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

* formatting (black)

as in CONTRIBUTING.md

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

* use connection open property instead of long chain through session

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

* trigger integration workflow

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

* fix: ensure open attribute of Connection never fails

in case the openSession takes long, the initialisation of the session
will not complete immediately. This could make the session attribute
inaccessible. If the Connection is deleted in this time, the open()
check will throw because the session attribute does not exist. Thus, we
default to the Connection being closed in this case. This was not an
issue before because open was a direct attribute of the Connection
class. Caught in the integration tests.

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

* fix: de-complicate earlier connection open logic

earlier, one of the integration tests was failing because 'session was
not an attribute of Connection'. This is likely tied to a local
configuration issue related to unittest that was causing an error in the
test suite itself. The tests are now passing without checking for the
session attribute.
c676f9b

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

* Revert "fix: de-complicate earlier connection open logic"

This reverts commit d6b1b19.

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

* [empty commit] attempt to trigger ci e2e workflow

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

* Update CODEOWNERS (#562)

new codeowners

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

* 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

* add

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

* PECOBLR-86 improve logging on python driver (#556)

* PECOBLR-86 Improve logging for debug level

Signed-off-by: Sai Shree Pradhan <[email protected]>

* PECOBLR-86 Improve logging for debug level

Signed-off-by: Sai Shree Pradhan <[email protected]>

* fixed format

Signed-off-by: Sai Shree Pradhan <[email protected]>

* used lazy logging

Signed-off-by: Sai Shree Pradhan <[email protected]>

* changed debug to error logs

Signed-off-by: Sai Shree Pradhan <[email protected]>

* used lazy logging

Signed-off-by: Sai Shree Pradhan <[email protected]>

---------

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

* Revert "Merge remote-tracking branch 'upstream/sea-migration' into decouple-session"

This reverts commit dbb2ec5, reversing
changes made to 7192f11.

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

* Reapply "Merge remote-tracking branch 'upstream/sea-migration' into decouple-session"

This reverts commit bdb8381.

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

* fix: separate session opening logic from instantiation

ensures correctness of self.session.open call in Connection

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

* fix: use is_open attribute to denote session availability

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

* fix: access thrift backend through session

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

* chore: use get_handle() instead of private session attribute in client

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

* formatting (black)

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

* fix: remove accidentally removed assertions

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

---------

Signed-off-by: varun-edachali-dbx <[email protected]>
Signed-off-by: Sai Shree Pradhan <[email protected]>
Co-authored-by: Jothi Prakash <[email protected]>
Co-authored-by: Madhav Sainanee <[email protected]>
Co-authored-by: Sai Shree Pradhan <[email protected]>
@varun-edachali-dbx varun-edachali-dbx mentioned this pull request Jun 18, 2025
5 tasks
varun-edachali-dbx added a commit that referenced this pull request Jul 15, 2025
* Separate Session related functionality from Connection class (#571)

* decouple session class from existing Connection

ensure maintenance of current APIs of Connection while delegating
responsibility

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

* add open property to Connection to ensure maintenance of existing API

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

* update unit tests to address ThriftBackend through session instead of through Connection

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

* chore: move session specific tests from test_client to test_session

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

* formatting (black)

as in CONTRIBUTING.md

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

* use connection open property instead of long chain through session

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

* trigger integration workflow

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

* fix: ensure open attribute of Connection never fails

in case the openSession takes long, the initialisation of the session
will not complete immediately. This could make the session attribute
inaccessible. If the Connection is deleted in this time, the open()
check will throw because the session attribute does not exist. Thus, we
default to the Connection being closed in this case. This was not an
issue before because open was a direct attribute of the Connection
class. Caught in the integration tests.

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

* fix: de-complicate earlier connection open logic

earlier, one of the integration tests was failing because 'session was
not an attribute of Connection'. This is likely tied to a local
configuration issue related to unittest that was causing an error in the
test suite itself. The tests are now passing without checking for the
session attribute.
c676f9b

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

* Revert "fix: de-complicate earlier connection open logic"

This reverts commit d6b1b19.

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

* [empty commit] attempt to trigger ci e2e workflow

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

* Update CODEOWNERS (#562)

new codeowners

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

* 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

* add

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

* PECOBLR-86 improve logging on python driver (#556)

* PECOBLR-86 Improve logging for debug level

Signed-off-by: Sai Shree Pradhan <[email protected]>

* PECOBLR-86 Improve logging for debug level

Signed-off-by: Sai Shree Pradhan <[email protected]>

* fixed format

Signed-off-by: Sai Shree Pradhan <[email protected]>

* used lazy logging

Signed-off-by: Sai Shree Pradhan <[email protected]>

* changed debug to error logs

Signed-off-by: Sai Shree Pradhan <[email protected]>

* used lazy logging

Signed-off-by: Sai Shree Pradhan <[email protected]>

---------

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

* Revert "Merge remote-tracking branch 'upstream/sea-migration' into decouple-session"

This reverts commit dbb2ec5, reversing
changes made to 7192f11.

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

* Reapply "Merge remote-tracking branch 'upstream/sea-migration' into decouple-session"

This reverts commit bdb8381.

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

* fix: separate session opening logic from instantiation

ensures correctness of self.session.open call in Connection

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

* fix: use is_open attribute to denote session availability

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

* fix: access thrift backend through session

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

* chore: use get_handle() instead of private session attribute in client

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

* formatting (black)

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

* fix: remove accidentally removed assertions

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

---------

Signed-off-by: varun-edachali-dbx <[email protected]>
Signed-off-by: Sai Shree Pradhan <[email protected]>
Co-authored-by: Jothi Prakash <[email protected]>
Co-authored-by: Madhav Sainanee <[email protected]>
Co-authored-by: Sai Shree Pradhan <[email protected]>

* Introduce Backend Interface (DatabricksClient) (#573)

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]>

* Implement ResultSet Abstraction (backend interfaces for fetch phase) (#574)

* ensure backend client returns a ResultSet type in backend tests

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

* formatting (black)

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]>

* stricter typing for cursor

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

* correct typing

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

* correct tests and merge artifacts

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

* remove accidentally modified workflow files

remnants of old merge

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

* chore: remove accidentally modified workflow files

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

* add back accidentally removed docstrings

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

* clean up docstrings

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

* log hex

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

* remove unnecessary _replace call

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

* add __str__ for CommandId

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

* take TOpenSessionResp in get_protocol_version to maintain existing interface

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

* active_op_handle -> active_mmand_id

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

* ensure None returned for close_command

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

* account for ResultSet return in new pydocs

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

* pydoc for types

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

* move common state to ResultSet aprent

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

* stronger typing in resultSet behaviour

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

* remove redundant patch in test

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

* add has_been_closed_server_side assertion

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

* remove redundancies in tests

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

* more robust close check

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

* use normalised state in e2e test

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

* simplify corrected test

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

* add line gaps after multi-line pydocs for consistency

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

* use normalised CommandState type in ExecuteResponse

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

---------

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

* remove un-necessary initialisation assertions

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

* remove un-necessary line break s

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

* more un-necessary line breaks

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

* constrain diff of test_closing_connection_closes_commands

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

* reduce diff of test_closing_connection_closes_commands

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

* use pytest-like assertions for test_closing_connection_closes_commands

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

* ensure command_id is not None

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

* line breaks after multi-line pyfocs

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

* ensure non null operationHandle for commandId creation

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

* use command_id methods instead of explicit guid_to_hex_id conversion

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

* remove un-necessary artifacts in test_session, add back assertion

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

* add from __future__ import annotations to remove string literals around forward refs, remove some unused imports

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

* move docstring of DatabricksClient within class

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

* move ThriftResultSet import to top of file

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

* make backend/utils __init__ file empty

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

* use from __future__ import annotations to remove string literals around Cursor

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

* use lazy logging

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

* replace getters with property tag

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

* set active_command_id to None, not active_op_handle

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

* align test_session with pytest instead of unittest

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

* formatting (black)

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

* remove repetition from Session.__init__

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

* mention that if catalog / schema name is None, we fetch across all

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

* mention fetching across all tables if null table name

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

* remove lazy import of ThriftResultSet

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

* remove unused import

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

* better docstrings

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

* clarified role of cursor in docstring

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

---------

Signed-off-by: varun-edachali-dbx <[email protected]>
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.

5 participants