Enhance Cursor close handling and context manager exception management to prevent server side resource leaks #554
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
madhav-db
requested review from
deeksha-db,
samikshya-db,
jprakash-db,
yunbodeng-db,
jackyhu-db and
benc-db
as code owners
|
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 ( |
|
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 ( |
|
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 ( |
madhav-db
changed the title
|
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 ( |
gopalldb
reviewed
May 14, 2025
gopalldb
approved these changes
May 14, 2025
jprakash-db
reviewed
May 15, 2025
jprakash-db
reviewed
May 15, 2025
…nsure proper closure during exceptions, including KeyboardInterrupt. Add tests for nested cursor management and verify operation closure on server-side errors.
|
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 ( |
|
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 ( |
|
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 ( |
jprakash-db
approved these changes
May 21, 2025
varun-edachali-dbx
pushed a commit
to varun-edachali-dbx/databricks-sql-python-dev
that referenced
this pull request
May 23, 2025
…t to prevent server side resource leaks (databricks#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 <varun.edachali@databricks.com>
varun-edachali-dbx
pushed a commit
to varun-edachali-dbx/databricks-sql-python-dev
that referenced
this pull request
May 24, 2025
…t to prevent server side resource leaks (databricks#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 <varun.edachali@databricks.com>
saishreeeee
pushed a commit
that referenced
this pull request
May 26, 2025
…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
saishreeeee
pushed a commit
that referenced
this pull request
May 26, 2025
…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: Sai Shree Pradhan <saishree.pradhan@databricks.com>
varun-edachali-dbx
pushed a commit
that referenced
this pull request
May 26, 2025
…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 <varun.edachali@databricks.com>
varun-edachali-dbx
pushed a commit
that referenced
this pull request
May 26, 2025
…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 <varun.edachali@databricks.com>
varun-edachali-dbx
pushed a commit
that referenced
this pull request
May 27, 2025
…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 <varun.edachali@databricks.com>
varun-edachali-dbx
pushed a commit
that referenced
this pull request
May 28, 2025
…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 <varun.edachali@databricks.com>
varun-edachali-dbx
pushed a commit
that referenced
this pull request
May 28, 2025
…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 <varun.edachali@databricks.com>
varun-edachali-dbx
pushed a commit
that referenced
this pull request
May 28, 2025
…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 <varun.edachali@databricks.com>
varun-edachali-dbx
added a commit
that referenced
this pull request
May 28, 2025
* decouple session class from existing Connection ensure maintenance of current APIs of Connection while delegating responsibility Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * add open property to Connection to ensure maintenance of existing API Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * update unit tests to address ThriftBackend through session instead of through Connection Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * chore: move session specific tests from test_client to test_session Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * formatting (black) as in CONTRIBUTING.md Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * use connection open property instead of long chain through session Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * trigger integration workflow Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * 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 <varun.edachali@databricks.com> * 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 <varun.edachali@databricks.com> * Revert "fix: de-complicate earlier connection open logic" This reverts commit d6b1b19. Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * [empty commit] attempt to trigger ci e2e workflow Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * Update CODEOWNERS (#562) new codeowners Signed-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 * add Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * PECOBLR-86 improve logging on python driver (#556) * PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed format Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * used lazy logging Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed debug to error logs Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * used lazy logging Signed-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 commit dbb2ec5, reversing changes made to 7192f11. Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * Reapply "Merge remote-tracking branch 'upstream/sea-migration' into decouple-session" This reverts commit bdb8381. Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * fix: separate session opening logic from instantiation ensures correctness of self.session.open call in Connection Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * fix: use is_open attribute to denote session availability Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * fix: access thrift backend through session Signed-off-by: varun-edachali-dbx <varun.edachali@databricks.com> * chore: use get_handle() instead of private session attribute in client Signed-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 assertions Signed-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>
saishreeeee
added a commit
that referenced
this pull request
May 30, 2025
* PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed format Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * used lazy logging Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed debug to error logs Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * added classes required for telemetry Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * removed TelemetryHelper Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * [PECOBLR-361] convert column table to arrow if arrow present (#551) Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * Update CODEOWNERS (#562) new codeowners Signed-off-by: Sai Shree Pradhan <saishree.pradhan@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 * add Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * PECOBLR-86 improve logging on python driver (#556) * PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed format Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * used lazy logging Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed debug to error logs Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * used lazy logging Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> --------- Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * Update github actions run conditions (#569) More conditions to run github actions Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * Added classes required for telemetry Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed example Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed to doc string Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * removed self.telemetry close line Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * grouped classes Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * formatting Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed doc string Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed doc string Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * added more descriptive comments, put dataclasses in a sub-folder Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed default attributes ordering Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed file names Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * added enums to models folder Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * removed telemetry batch size Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> --------- Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> Co-authored-by: Shivam Raj <171748731+shivam2680@users.noreply.github.com> Co-authored-by: Jothi Prakash <jothi.prakash@databricks.com> Co-authored-by: Madhav Sainanee <madhav.sainanee@databricks.com>
saishreeeee
pushed a commit
that referenced
this pull request
Jun 4, 2025
…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
saishreeeee
added a commit
that referenced
this pull request
Jun 4, 2025
* PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed format Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * used lazy logging Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed debug to error logs Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * added classes required for telemetry Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * removed TelemetryHelper Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * [PECOBLR-361] convert column table to arrow if arrow present (#551) Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * Update CODEOWNERS (#562) new codeowners Signed-off-by: Sai Shree Pradhan <saishree.pradhan@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 * add Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * PECOBLR-86 improve logging on python driver (#556) * PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed format Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * used lazy logging Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed debug to error logs Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * used lazy logging Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> --------- Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * Update github actions run conditions (#569) More conditions to run github actions Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * Added classes required for telemetry Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed example Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed to doc string Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * removed self.telemetry close line Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * grouped classes Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * formatting Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed doc string Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed doc string Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * added more descriptive comments, put dataclasses in a sub-folder Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed default attributes ordering Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed file names Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * added enums to models folder Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * removed telemetry batch size Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> --------- Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> Co-authored-by: Shivam Raj <171748731+shivam2680@users.noreply.github.com> Co-authored-by: Jothi Prakash <jothi.prakash@databricks.com> Co-authored-by: Madhav Sainanee <madhav.sainanee@databricks.com>
saishreeeee
pushed a commit
that referenced
this pull request
Jun 4, 2025
…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: Sai Shree Pradhan <saishree.pradhan@databricks.com>
saishreeeee
added a commit
that referenced
this pull request
Jun 4, 2025
* PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed format Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * used lazy logging Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed debug to error logs Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * added classes required for telemetry Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * removed TelemetryHelper Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * [PECOBLR-361] convert column table to arrow if arrow present (#551) Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * Update CODEOWNERS (#562) new codeowners Signed-off-by: Sai Shree Pradhan <saishree.pradhan@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 * add Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * PECOBLR-86 improve logging on python driver (#556) * PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * PECOBLR-86 Improve logging for debug level Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed format Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * used lazy logging Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed debug to error logs Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * used lazy logging Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> --------- Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * Update github actions run conditions (#569) More conditions to run github actions Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * Added classes required for telemetry Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed example Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed to doc string Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * removed self.telemetry close line Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * grouped classes Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * formatting Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed doc string Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed doc string Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * added more descriptive comments, put dataclasses in a sub-folder Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * fixed default attributes ordering Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * changed file names Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * added enums to models folder Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> * removed telemetry batch size Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> --------- Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com> Co-authored-by: Shivam Raj <171748731+shivam2680@users.noreply.github.com> Co-authored-by: Jothi Prakash <jothi.prakash@databricks.com> Co-authored-by: Madhav Sainanee <madhav.sainanee@databricks.com> Signed-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
Description
This PR attempts to fix an issue where database connections were not properly closing, resulting in "lame duck" sessions that kept SQL warehouses active unnecessarily.
When clients disconnected, operation handles were not properly being closed on the server side, potentially leading to resource leaks. The
Cursor.close()methodwas simply nullifying the operation handle reference without properly closing it on the thrift backend.Besides this, in
Cursor.exit(), added better error handling to prevent exceptions during cursor closure from suppressing original exceptions, which is particularly important for user introduced interruptions (eg. Keyboard interrupt). Before this fix, when users interrupted long-running queries, the exception from cursor.close() could mask the KeyboardInterrupt, preventing the application from properly responding to cancellation requests and potentially leaving resources uncleaned.How is this tested?
Added new unit tests to verify behaviour
Related Tickets & Documents