From 5541c942232735bc07f9ae9cdf13f12ceed565e8 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" <3815206+peco-engineer-bot[bot]@users.noreply.github.com> Date: Thu, 23 Jul 2026 05:57:39 +0000 Subject: [PATCH 1/2] Session is not set in object if connection fails, which raises 'AttributeError' on __del__ (#746) Signed-off-by: peco-engineer-bot[bot] <3815206+peco-engineer-bot[bot]@users.noreply.github.com> --- src/databricks/sql/client.py | 6 ++++++ tests/e2e/test_driver.py | 40 ++++++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/src/databricks/sql/client.py b/src/databricks/sql/client.py index d45d51181..8b4339a8c 100755 --- a/src/databricks/sql/client.py +++ b/src/databricks/sql/client.py @@ -462,6 +462,12 @@ def __exit__(self, exc_type, exc_value, traceback): self.close() def __del__(self): + # If construction failed before ``self.session`` was assigned (e.g. the + # Session constructor raised), there is nothing to close. Guard against + # the missing attribute so __del__ doesn't raise during garbage + # collection. See https://github.com/databricks/databricks-sql-python/issues/746 + if not hasattr(self, "session"): + return if self.open: logger.debug( "Closing unclosed connection for session " diff --git a/tests/e2e/test_driver.py b/tests/e2e/test_driver.py index 5fe3db037..9694ef8cf 100644 --- a/tests/e2e/test_driver.py +++ b/tests/e2e/test_driver.py @@ -2,6 +2,7 @@ from contextlib import contextmanager from collections import OrderedDict import datetime +import gc import io import logging import os @@ -1155,6 +1156,45 @@ def test_row_limit_with_arrow_smaller_result(self): # use a RetrySuite to encapsulate these tests which we'll typically want to run together; however keep # the 429/503 subsuites separate since they execute under different circumstances. +class TestPySQLConnectionFailureSuite(PySQLPytestTestCase): + """Regression tests for connection construction failures (issue #746).""" + + def test_del_does_not_raise_when_session_never_set(self): + """A constructor-stage connection failure must not leave __del__ raising + AttributeError when the half-constructed Connection is garbage-collected. + + Username/password auth raises ValueError inside the Session constructor, + i.e. before ``self.session`` is assigned in Connection.__init__. The + original connection error must propagate, and __del__ on the + half-constructed Connection must not raise (which Python would report as + an 'Exception ignored in __del__' referencing the missing ``session`` + attribute). + """ + # Use the live-warehouse connection params, but inject username/password + # which triggers a client-side ValueError inside Session.__init__. + params = dict(self.connection_params(), username="user", password="pass") + + unraisable = [] + original_hook = sys.unraisablehook + sys.unraisablehook = lambda unraisable_hook_args: unraisable.append( + unraisable_hook_args + ) + try: + with pytest.raises(ValueError): + sql.connect(**params) + + # Force collection of the half-constructed Connection so __del__ runs. + gc.collect() + finally: + sys.unraisablehook = original_hook + + messages = [ + "{}: {}".format(type(u.exc_value).__name__, u.exc_value) + for u in unraisable + ] + assert not unraisable, "Exception(s) raised in __del__: {}".format(messages) + + class TestPySQLRetrySuite: class HTTP429Suite(Client429ResponseMixin, PySQLPytestTestCase): pass # Mixin covers all From bc07abfecae3e6e8fc9937b2e42bcc63ab0d5750 Mon Sep 17 00:00:00 2001 From: "peco-engineer-bot[bot]" Date: Thu, 23 Jul 2026 06:04:09 +0000 Subject: [PATCH 2/2] ai: apply changes for #877 (1 review thread) Addresses: - #3635886434 at tests/e2e/test_driver.py:1159 Signed-off-by: peco-engineer-bot[bot] --- tests/unit/test_client.py | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/tests/unit/test_client.py b/tests/unit/test_client.py index 66a3722ec..7f938bfa1 100644 --- a/tests/unit/test_client.py +++ b/tests/unit/test_client.py @@ -90,6 +90,38 @@ class ClientTestSuite(unittest.TestCase): "enable_telemetry": False, } + @patch("%s.client.Session" % PACKAGE_NAME) + def test_del_does_not_raise_when_session_never_set(self, mock_session_class): + """A constructor-stage failure must not leave __del__ raising on GC. + + When the Session constructor raises before ``self.session`` is assigned + in Connection.__init__, the original error must propagate, and __del__ on + the half-constructed Connection must not raise (which Python reports as an + 'Exception ignored in __del__' referencing the missing ``session`` + attribute). See https://github.com/databricks/databricks-sql-python/issues/746 + """ + mock_session_class.side_effect = ValueError("bad auth") + + unraisable = [] + original_hook = sys.unraisablehook + sys.unraisablehook = lambda args: unraisable.append(args) + try: + with self.assertRaises(ValueError): + client.Connection(**self.DUMMY_CONNECTION_ARGS) + + # Force collection of the half-constructed Connection so __del__ runs. + gc.collect() + finally: + sys.unraisablehook = original_hook + + messages = [ + "{}: {}".format(type(u.exc_value).__name__, u.exc_value) + for u in unraisable + ] + self.assertEqual( + unraisable, [], "Exception(s) raised in __del__: {}".format(messages) + ) + @patch("%s.session.ThriftDatabricksClient" % PACKAGE_NAME) def test_closing_connection_closes_commands(self, mock_thrift_client_class): """Test that closing a connection properly closes commands.