From 8351df900019b6393ad16f4d1ccbe137ea7045be Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 23 Oct 2020 11:58:35 +0300 Subject: [PATCH 1/4] feat: clear session pool on connection close --- google/cloud/spanner_dbapi/connection.py | 1 + tests/spanner_dbapi/test_connection.py | 8 ++++++++ 2 files changed, 9 insertions(+) diff --git a/google/cloud/spanner_dbapi/connection.py b/google/cloud/spanner_dbapi/connection.py index 8907e65c03..b4a6c84319 100644 --- a/google/cloud/spanner_dbapi/connection.py +++ b/google/cloud/spanner_dbapi/connection.py @@ -235,6 +235,7 @@ def close(self): ): self._transaction.rollback() + self.database._pool.clear() self.is_closed = True def commit(self): diff --git a/tests/spanner_dbapi/test_connection.py b/tests/spanner_dbapi/test_connection.py index 24260de12e..5145346902 100644 --- a/tests/spanner_dbapi/test_connection.py +++ b/tests/spanner_dbapi/test_connection.py @@ -77,3 +77,11 @@ def test_instance_property(self): with self.assertRaises(AttributeError): connection.instance = None + + def test_clearing_pool_on_close(self): + connection = self._make_connection() + with mock.patch.object( + connection.database._pool, "clear" + ) as pool_clear_mock: + connection.close() + pool_clear_mock.assert_called_once() From 9b1ca23aa754da265b97a5d43d5a1f8b5b001103 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 3 Nov 2020 11:36:38 +0300 Subject: [PATCH 2/4] use protected property --- google/cloud/spanner_dbapi/connection.py | 14 ++++++++-- tests/spanner_dbapi/test_connection.py | 8 ------ tests/unit/spanner_dbapi/test_connection.py | 29 +++++++++++++++++---- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/google/cloud/spanner_dbapi/connection.py b/google/cloud/spanner_dbapi/connection.py index f8d11f4e7f..beb05a3173 100644 --- a/google/cloud/spanner_dbapi/connection.py +++ b/google/cloud/spanner_dbapi/connection.py @@ -43,6 +43,10 @@ def __init__(self, instance, database): self.is_closed = False self._autocommit = False + # indicator to know if the session pool used by + # this connection should be cleared on the + # connection close + self._own_pool = True @property def autocommit(self): @@ -150,7 +154,9 @@ def close(self): ): self._transaction.rollback() - self.database._pool.clear() + if self._own_pool: + self.database._pool.clear() + self.is_closed = True def commit(self): @@ -259,4 +265,8 @@ def connect( if not database.exists(): raise ValueError("database '%s' does not exist." % database_id) - return Connection(instance, database) + conn = Connection(instance, database) + if pool is not None: + conn._own_pool = False + + return conn diff --git a/tests/spanner_dbapi/test_connection.py b/tests/spanner_dbapi/test_connection.py index 5145346902..24260de12e 100644 --- a/tests/spanner_dbapi/test_connection.py +++ b/tests/spanner_dbapi/test_connection.py @@ -77,11 +77,3 @@ def test_instance_property(self): with self.assertRaises(AttributeError): connection.instance = None - - def test_clearing_pool_on_close(self): - connection = self._make_connection() - with mock.patch.object( - connection.database._pool, "clear" - ) as pool_clear_mock: - connection.close() - pool_clear_mock.assert_called_once() diff --git a/tests/unit/spanner_dbapi/test_connection.py b/tests/unit/spanner_dbapi/test_connection.py index d545472c57..d76a6bd889 100644 --- a/tests/unit/spanner_dbapi/test_connection.py +++ b/tests/unit/spanner_dbapi/test_connection.py @@ -34,14 +34,19 @@ def _get_client_info(self): return ClientInfo(user_agent=self.USER_AGENT) - def _make_connection(self): + def _make_connection(self, pool=None): from google.cloud.spanner_dbapi import Connection from google.cloud.spanner_v1.instance import Instance # We don't need a real Client object to test the constructor instance = Instance(self.INSTANCE, client=None) database = instance.database(self.DATABASE) - return Connection(instance, database) + + conn = Connection(instance, database) + if pool is not None: + conn._own_pool = False + + return conn def test_property_autocommit_setter(self): from google.cloud.spanner_dbapi import Connection @@ -239,9 +244,7 @@ def test_run_prior_DDL_statements(self): connection.run_prior_DDL_statements() def test_context(self): - from google.cloud.spanner_dbapi import Connection - - connection = Connection(self.INSTANCE, self.DATABASE) + connection = self._make_connection() with connection as conn: self.assertEqual(conn, connection) @@ -316,3 +319,19 @@ def test_sessions_pool(self): ): connect("test-instance", database_id, pool=pool) database_mock.assert_called_once_with(database_id, pool=pool) + + def test_clearing_pool_on_close(self): + connection = self._make_connection() + with mock.patch.object( + connection.database._pool, "clear" + ) as pool_clear_mock: + connection.close() + pool_clear_mock.assert_called_once() + + def test_global_pool(self): + connection = self._make_connection(pool=mock.Mock()) + with mock.patch.object( + connection.database._pool, "clear" + ) as pool_clear_mock: + connection.close() + pool_clear_mock.assert_not_called() From 05d0fd58391f299deff13f28ab6f12de89def9f8 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 3 Nov 2020 11:45:58 +0300 Subject: [PATCH 3/4] fix tests for Python 3.5 --- tests/unit/spanner_dbapi/test_connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/spanner_dbapi/test_connection.py b/tests/unit/spanner_dbapi/test_connection.py index d76a6bd889..85ff637117 100644 --- a/tests/unit/spanner_dbapi/test_connection.py +++ b/tests/unit/spanner_dbapi/test_connection.py @@ -334,4 +334,4 @@ def test_global_pool(self): connection.database._pool, "clear" ) as pool_clear_mock: connection.close() - pool_clear_mock.assert_not_called() + assert not pool_clear_mock.called From 8e9180e56c64c16d4aea8cc3c8c099af5c6fde22 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Tue, 3 Nov 2020 11:56:09 +0300 Subject: [PATCH 4/4] fix tests for Python 3.5 --- tests/unit/spanner_dbapi/test_connection.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/spanner_dbapi/test_connection.py b/tests/unit/spanner_dbapi/test_connection.py index 85ff637117..99aa0aa47b 100644 --- a/tests/unit/spanner_dbapi/test_connection.py +++ b/tests/unit/spanner_dbapi/test_connection.py @@ -326,7 +326,7 @@ def test_clearing_pool_on_close(self): connection.database._pool, "clear" ) as pool_clear_mock: connection.close() - pool_clear_mock.assert_called_once() + pool_clear_mock.assert_called_once_with() def test_global_pool(self): connection = self._make_connection(pool=mock.Mock())