Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Session is not set in object if connection fails, which raises 'Attri…
…buteError' on __del__ (#746)

Signed-off-by: peco-engineer-bot[bot] <3815206+peco-engineer-bot[bot]@users.noreply.github.com>
  • Loading branch information
peco-engineer-bot[bot]
peco-engineer-bot[bot] committed Jul 23, 2026
commit 5541c942232735bc07f9ae9cdf13f12ceed565e8
6 changes: 6 additions & 0 deletions src/databricks/sql/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
40 changes: 40 additions & 0 deletions tests/e2e/test_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from contextlib import contextmanager
from collections import OrderedDict
import datetime
import gc
import io
import logging
import os
Expand Down Expand Up @@ -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):
Comment thread
peco-review-bot[bot] marked this conversation as resolved.
"""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
Expand Down
Loading