Skip to content

Commit 02616cf

Browse files
fixup! NO-SNOW: Add thread-safe pool for tracking opened connections
1 parent a6369fd commit 02616cf

File tree

2 files changed

+25
-20
lines changed

2 files changed

+25
-20
lines changed

src/snowflake/connector/connection.py

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,15 +1154,15 @@ def connect(self, **kwargs) -> None:
11541154
self.__open_connection()
11551155

11561156
# Register the connection in the pool after successful connection
1157-
_connections_pool.add_connection(self)
1157+
_connections_registry.add_connection(self)
11581158

11591159
def close(self, retry: bool = True) -> None:
11601160
"""Closes the connection."""
11611161
# unregister to dereference connection object as it's already closed after the execution
11621162
atexit.unregister(self._close_at_exit)
11631163
try:
11641164
# Remove connection from the pool
1165-
_connections_pool.remove_connection(self)
1165+
_connections_registry.remove_connection(self)
11661166

11671167
if not self.rest:
11681168
logger.debug("Rest object has been destroyed, cannot close session")
@@ -2542,20 +2542,20 @@ def _detect_application() -> None | str:
25422542
return "snowflake_notebook"
25432543

25442544

2545-
class _ConnectionsPool:
2546-
"""Thread-safe pool for tracking opened SnowflakeConnection instances.
2545+
class _ConnectionsRegistry:
2546+
"""Thread-safe registry for tracking opened SnowflakeConnection instances.
25472547
25482548
This class maintains a registry of active connections using weak references
25492549
to avoid preventing garbage collection.
25502550
"""
25512551

25522552
def __init__(self):
2553-
"""Initialize the connections pool with an empty registry and a lock."""
2553+
"""Initialize the connections registry with an empty registry and a lock."""
25542554
self._connections: weakref.WeakSet = weakref.WeakSet()
25552555
self._lock = Lock()
25562556

25572557
def add_connection(self, connection: SnowflakeConnection) -> None:
2558-
"""Add a connection to the pool.
2558+
"""Add a connection to the registry.
25592559
25602560
Args:
25612561
connection: The SnowflakeConnection instance to register.
@@ -2567,26 +2567,29 @@ def add_connection(self, connection: SnowflakeConnection) -> None:
25672567
)
25682568

25692569
def remove_connection(self, connection: SnowflakeConnection) -> None:
2570-
"""Remove a connection from the pool.
2570+
"""Remove a connection from the registry.
25712571
25722572
Args:
25732573
connection: The SnowflakeConnection instance to unregister.
25742574
"""
25752575
with self._lock:
25762576
self._connections.discard(connection)
25772577
logger.debug(
2578-
f"Connection {id(connection)} removed from pool. Total connections: {len(self._connections)}"
2578+
f"Connection {id(connection)} removed from registry. Total connections: {len(self._connections)}"
25792579
)
25802580

25812581
if len(self._connections) == 0:
2582-
# If no connections left then stop CRL background task
2583-
# to avoid script dangling
2584-
CRLCacheFactory.stop_periodic_cleanup()
2582+
self._last_connection_handler()
2583+
2584+
def _last_connection_handler(self):
2585+
# If no connections left then stop CRL background task
2586+
# to avoid script dangling
2587+
CRLCacheFactory.stop_periodic_cleanup()
25852588

25862589
def get_connection_count(self) -> int:
25872590
with self._lock:
25882591
return len(self._connections)
25892592

25902593

25912594
# Global instance of the connections pool
2592-
_connections_pool = _ConnectionsPool()
2595+
_connections_registry = _ConnectionsRegistry()

test/unit/test_connection.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -955,41 +955,42 @@ def test_connect_metadata_preservation():
955955
# Should have parameters like account, user, password, etc.
956956

957957

958-
def test_connections_pool(mock_post_requests):
958+
def test_connections_registry(mock_post_requests):
959959
"""Test that connections are properly tracked in the _ConnectionsPool."""
960-
from snowflake.connector.connection import _connections_pool
960+
from snowflake.connector.connection import _connections_registry
961961

962962
# Get initial connection count
963-
initial_count = _connections_pool.get_connection_count()
963+
initial_count = _connections_registry.get_connection_count()
964964

965965
# Create and connect first connection
966966
conn1 = fake_connector()
967967
assert (
968-
_connections_pool.get_connection_count() == initial_count + 1
968+
_connections_registry.get_connection_count() == initial_count + 1
969969
), "Connection count should increase by 1 after creating a connection"
970970

971971
# Create and connect second connection
972972
conn2 = fake_connector()
973973
assert (
974-
_connections_pool.get_connection_count() == initial_count + 2
974+
_connections_registry.get_connection_count() == initial_count + 2
975975
), "Connection count should increase by 2 after creating two connections"
976976

977977
# Close first connection
978978
conn1.close()
979979
assert (
980-
_connections_pool.get_connection_count() == initial_count + 1
980+
_connections_registry.get_connection_count() == initial_count + 1
981981
), "Connection count should decrease by 1 after closing a connection"
982982

983983
# Close second connection
984984
conn2.close()
985985
assert (
986-
_connections_pool.get_connection_count() == initial_count
986+
_connections_registry.get_connection_count() == initial_count
987987
), "Connection count should return to initial count after closing all connections"
988988

989989

990990
@mock.patch("snowflake.connector.connection.CRLCacheFactory")
991-
def test_connections_pool_stops_crl_task_if_empty(crl_mock, mock_post_requests):
991+
def test_connections_registry_stops_crl_task_if_empty(crl_mock, mock_post_requests):
992992
"""Test the individual methods of _ConnectionsPool."""
993+
from snowflake.connector.connection import _connections_registry
993994

994995
# Create a connection
995996
conn1 = fake_connector()
@@ -1001,4 +1002,5 @@ def test_connections_pool_stops_crl_task_if_empty(crl_mock, mock_post_requests):
10011002

10021003
# Stop the task if the pool is emptied
10031004
conn2.close()
1005+
assert _connections_registry.get_connection_count() == 0
10041006
crl_mock.stop_periodic_cleanup.assert_called_once()

0 commit comments

Comments
 (0)