Skip to content

Commit 5541c94

Browse files
author
peco-engineer-bot[bot]
committed
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>
1 parent 9775996 commit 5541c94

2 files changed

Lines changed: 46 additions & 0 deletions

File tree

src/databricks/sql/client.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,12 @@ def __exit__(self, exc_type, exc_value, traceback):
462462
self.close()
463463

464464
def __del__(self):
465+
# If construction failed before ``self.session`` was assigned (e.g. the
466+
# Session constructor raised), there is nothing to close. Guard against
467+
# the missing attribute so __del__ doesn't raise during garbage
468+
# collection. See https://github.com/databricks/databricks-sql-python/issues/746
469+
if not hasattr(self, "session"):
470+
return
465471
if self.open:
466472
logger.debug(
467473
"Closing unclosed connection for session "

tests/e2e/test_driver.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from contextlib import contextmanager
33
from collections import OrderedDict
44
import datetime
5+
import gc
56
import io
67
import logging
78
import os
@@ -1155,6 +1156,45 @@ def test_row_limit_with_arrow_smaller_result(self):
11551156

11561157
# use a RetrySuite to encapsulate these tests which we'll typically want to run together; however keep
11571158
# the 429/503 subsuites separate since they execute under different circumstances.
1159+
class TestPySQLConnectionFailureSuite(PySQLPytestTestCase):
1160+
"""Regression tests for connection construction failures (issue #746)."""
1161+
1162+
def test_del_does_not_raise_when_session_never_set(self):
1163+
"""A constructor-stage connection failure must not leave __del__ raising
1164+
AttributeError when the half-constructed Connection is garbage-collected.
1165+
1166+
Username/password auth raises ValueError inside the Session constructor,
1167+
i.e. before ``self.session`` is assigned in Connection.__init__. The
1168+
original connection error must propagate, and __del__ on the
1169+
half-constructed Connection must not raise (which Python would report as
1170+
an 'Exception ignored in __del__' referencing the missing ``session``
1171+
attribute).
1172+
"""
1173+
# Use the live-warehouse connection params, but inject username/password
1174+
# which triggers a client-side ValueError inside Session.__init__.
1175+
params = dict(self.connection_params(), username="user", password="pass")
1176+
1177+
unraisable = []
1178+
original_hook = sys.unraisablehook
1179+
sys.unraisablehook = lambda unraisable_hook_args: unraisable.append(
1180+
unraisable_hook_args
1181+
)
1182+
try:
1183+
with pytest.raises(ValueError):
1184+
sql.connect(**params)
1185+
1186+
# Force collection of the half-constructed Connection so __del__ runs.
1187+
gc.collect()
1188+
finally:
1189+
sys.unraisablehook = original_hook
1190+
1191+
messages = [
1192+
"{}: {}".format(type(u.exc_value).__name__, u.exc_value)
1193+
for u in unraisable
1194+
]
1195+
assert not unraisable, "Exception(s) raised in __del__: {}".format(messages)
1196+
1197+
11581198
class TestPySQLRetrySuite:
11591199
class HTTP429Suite(Client429ResponseMixin, PySQLPytestTestCase):
11601200
pass # Mixin covers all

0 commit comments

Comments
 (0)