From f98efabf896298f8c7bd056112a7e16576442a43 Mon Sep 17 00:00:00 2001 From: kgala2 Date: Mon, 7 Apr 2025 15:23:59 -0700 Subject: [PATCH 1/9] chore: add check for closed connector --- google/cloud/sql/connector/connector.py | 7 ++++ tests/unit/test_connector.py | 55 ++++++++++++++++++------- 2 files changed, 46 insertions(+), 16 deletions(-) diff --git a/google/cloud/sql/connector/connector.py b/google/cloud/sql/connector/connector.py index 05eaa51df..2a81baff8 100755 --- a/google/cloud/sql/connector/connector.py +++ b/google/cloud/sql/connector/connector.py @@ -153,6 +153,7 @@ def __init__( # connection name string and enable_iam_auth boolean flag self._cache: dict[tuple[str, bool], MonitoredCache] = {} self._client: Optional[CloudSQLClient] = None + self._closed: bool = False # initialize credentials scopes = ["https://www.googleapis.com/auth/sqlservice.admin"] @@ -279,7 +280,11 @@ async def connect_async( and then subsequent attempt with IAM database authentication. KeyError: Unsupported database driver Must be one of pymysql, asyncpg, pg8000, and pytds. + RuntimeError: Connector has been closed. Cannot connect using a closed + Connector. """ + if self._closed: + raise RuntimeError("Cannot connect using a closed Connector.") if self._keys is None: self._keys = asyncio.create_task(generate_keys()) if self._client is None: @@ -462,6 +467,7 @@ def close(self) -> None: self._loop.call_soon_threadsafe(self._loop.stop) # wait for thread to finish closing (i.e. loop to stop) self._thread.join() + self._closed = True async def close_async(self) -> None: """Helper function to cancel the cache's tasks @@ -469,6 +475,7 @@ async def close_async(self) -> None: await asyncio.gather(*[cache.close() for cache in self._cache.values()]) if self._client: await self._client.close() + self._closed = True async def create_async_connector( diff --git a/tests/unit/test_connector.py b/tests/unit/test_connector.py index 157697723..e76c92281 100644 --- a/tests/unit/test_connector.py +++ b/tests/unit/test_connector.py @@ -1,19 +1,3 @@ -""" -Copyright 2021 Google LLC - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - https://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -""" - import asyncio import os from typing import Union @@ -468,3 +452,42 @@ def test_configured_quota_project_env_var( assert connector._quota_project == quota_project # unset env var del os.environ["GOOGLE_CLOUD_QUOTA_PROJECT"] + + +@pytest.mark.asyncio +async def test_connect_async_closed_connector( + fake_credentials: Credentials, fake_client: CloudSQLClient +) -> None: + """Test that calling connect_async() on a closed connector raises an error.""" + async with Connector( + credentials=fake_credentials, loop=asyncio.get_running_loop() + ) as connector: + connector._client = fake_client + await connector.close_async() + with pytest.raises(RuntimeError) as exc_info: + await connector.connect_async( + "test-project:test-region:test-instance", + "asyncpg", + user="my-user", + password="my-pass", + db="my-db", + ) + assert exc_info.value.args[0] == "Cannot connect using a closed Connector." + + +def test_connect_closed_connector( + fake_credentials: Credentials, fake_client: CloudSQLClient +) -> None: + """Test that calling connect() on a closed connector raises an error.""" + with Connector(credentials=fake_credentials) as connector: + connector._client = fake_client + connector.close() + with pytest.raises(RuntimeError) as exc_info: + connector.connect( + "test-project:test-region:test-instance", + "pg8000", + user="my-user", + password="my-pass", + db="my-db", + ) + assert exc_info.value.args[0] == "Cannot connect using a closed Connector." From ad46fd7f011937d46ca5d479ab8ec0216bd5e490 Mon Sep 17 00:00:00 2001 From: kgala2 Date: Mon, 7 Apr 2025 15:30:28 -0700 Subject: [PATCH 2/9] chore: add header back to the test_connector.py --- tests/unit/test_connector.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/unit/test_connector.py b/tests/unit/test_connector.py index e76c92281..544940662 100644 --- a/tests/unit/test_connector.py +++ b/tests/unit/test_connector.py @@ -1,3 +1,19 @@ +""" +Copyright 2021 Google LLC + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +""" + import asyncio import os from typing import Union From 7799b12fe60f68cbc0ab32b0a65cd8dd38f3e2c3 Mon Sep 17 00:00:00 2001 From: kgala2 Date: Mon, 7 Apr 2025 15:44:44 -0700 Subject: [PATCH 3/9] chore: add await in aync connector test --- tests/unit/test_connector.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/test_connector.py b/tests/unit/test_connector.py index 544940662..1bbe62013 100644 --- a/tests/unit/test_connector.py +++ b/tests/unit/test_connector.py @@ -480,6 +480,8 @@ async def test_connect_async_closed_connector( ) as connector: connector._client = fake_client await connector.close_async() + # wait for close to complete + await asyncio.sleep(0.1) with pytest.raises(RuntimeError) as exc_info: await connector.connect_async( "test-project:test-region:test-instance", From b1e046b981bcbb63fac5c9ec2bc15681438592dc Mon Sep 17 00:00:00 2001 From: kgala2 Date: Mon, 7 Apr 2025 15:56:48 -0700 Subject: [PATCH 4/9] chore: try adding time.sleep for sync closed connector --- tests/unit/test_connector.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/test_connector.py b/tests/unit/test_connector.py index 1bbe62013..ed8683dd6 100644 --- a/tests/unit/test_connector.py +++ b/tests/unit/test_connector.py @@ -16,6 +16,7 @@ import asyncio import os +import time from typing import Union from aiohttp import ClientResponseError @@ -500,6 +501,7 @@ def test_connect_closed_connector( with Connector(credentials=fake_credentials) as connector: connector._client = fake_client connector.close() + time.sleep(0.1) with pytest.raises(RuntimeError) as exc_info: connector.connect( "test-project:test-region:test-instance", From 9a192a4804e6cd415c454195caab808a1e4254c7 Mon Sep 17 00:00:00 2001 From: kgala2 Date: Tue, 15 Apr 2025 12:31:00 -0700 Subject: [PATCH 5/9] chore: update closed conn tests --- google/cloud/sql/connector/connector.py | 1 - tests/unit/test_connector.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/google/cloud/sql/connector/connector.py b/google/cloud/sql/connector/connector.py index 2a81baff8..636a87a90 100755 --- a/google/cloud/sql/connector/connector.py +++ b/google/cloud/sql/connector/connector.py @@ -467,7 +467,6 @@ def close(self) -> None: self._loop.call_soon_threadsafe(self._loop.stop) # wait for thread to finish closing (i.e. loop to stop) self._thread.join() - self._closed = True async def close_async(self) -> None: """Helper function to cancel the cache's tasks diff --git a/tests/unit/test_connector.py b/tests/unit/test_connector.py index ed8683dd6..89b9e909f 100644 --- a/tests/unit/test_connector.py +++ b/tests/unit/test_connector.py @@ -501,7 +501,7 @@ def test_connect_closed_connector( with Connector(credentials=fake_credentials) as connector: connector._client = fake_client connector.close() - time.sleep(0.1) + time.sleep(3.1) with pytest.raises(RuntimeError) as exc_info: connector.connect( "test-project:test-region:test-instance", From 27c2da800d31fb534653f03e53b8721761d4899e Mon Sep 17 00:00:00 2001 From: kgala2 Date: Fri, 16 May 2025 14:59:51 -0700 Subject: [PATCH 6/9] chore: update _closed internal variable in connect function --- google/cloud/sql/connector/connector.py | 3 +++ tests/unit/test_connector.py | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/google/cloud/sql/connector/connector.py b/google/cloud/sql/connector/connector.py index 636a87a90..adb8d4474 100755 --- a/google/cloud/sql/connector/connector.py +++ b/google/cloud/sql/connector/connector.py @@ -243,6 +243,8 @@ def connect( # connect runs sync database connections on background thread. # Async database connections should call 'connect_async' directly to # avoid hanging indefinitely. + if self._closed: + raise RuntimeError("Cannot connect using a closed Connector.") connect_future = asyncio.run_coroutine_threadsafe( self.connect_async(instance_connection_string, driver, **kwargs), self._loop, @@ -467,6 +469,7 @@ def close(self) -> None: self._loop.call_soon_threadsafe(self._loop.stop) # wait for thread to finish closing (i.e. loop to stop) self._thread.join() + self._closed = True async def close_async(self) -> None: """Helper function to cancel the cache's tasks diff --git a/tests/unit/test_connector.py b/tests/unit/test_connector.py index 89b9e909f..cd4746d3e 100644 --- a/tests/unit/test_connector.py +++ b/tests/unit/test_connector.py @@ -482,7 +482,7 @@ async def test_connect_async_closed_connector( connector._client = fake_client await connector.close_async() # wait for close to complete - await asyncio.sleep(0.1) + # await asyncio.sleep(0.1) with pytest.raises(RuntimeError) as exc_info: await connector.connect_async( "test-project:test-region:test-instance", @@ -501,7 +501,7 @@ def test_connect_closed_connector( with Connector(credentials=fake_credentials) as connector: connector._client = fake_client connector.close() - time.sleep(3.1) + # time.sleep(3.1) with pytest.raises(RuntimeError) as exc_info: connector.connect( "test-project:test-region:test-instance", From e76133f0f7b84dccbf947bfbf250f7324201dfc2 Mon Sep 17 00:00:00 2001 From: kgala2 Date: Fri, 16 May 2025 15:25:03 -0700 Subject: [PATCH 7/9] chore: update exception type and exception message --- google/cloud/sql/connector/connector.py | 11 +++++++++-- google/cloud/sql/connector/exceptions.py | 7 +++++++ tests/unit/test_connector.py | 19 +++++++++++-------- 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/google/cloud/sql/connector/connector.py b/google/cloud/sql/connector/connector.py index adb8d4474..a674f25b6 100755 --- a/google/cloud/sql/connector/connector.py +++ b/google/cloud/sql/connector/connector.py @@ -43,6 +43,7 @@ from google.cloud.sql.connector.resolver import DnsResolver from google.cloud.sql.connector.utils import format_database_user from google.cloud.sql.connector.utils import generate_keys +from google.cloud.sql.connector.exceptions import ClosedConnectorError logger = logging.getLogger(name=__name__) @@ -243,8 +244,12 @@ def connect( # connect runs sync database connections on background thread. # Async database connections should call 'connect_async' directly to # avoid hanging indefinitely. + + # Check if the connector is closed before attempting to connect. if self._closed: - raise RuntimeError("Cannot connect using a closed Connector.") + raise ClosedConnectorError( + "Connection attempt failed because the connector has already been closed." + ) connect_future = asyncio.run_coroutine_threadsafe( self.connect_async(instance_connection_string, driver, **kwargs), self._loop, @@ -286,7 +291,9 @@ async def connect_async( Connector. """ if self._closed: - raise RuntimeError("Cannot connect using a closed Connector.") + raise ClosedConnectorError( + "Connection attempt failed because the connector has already been closed." + ) if self._keys is None: self._keys = asyncio.create_task(generate_keys()) if self._client is None: diff --git a/google/cloud/sql/connector/exceptions.py b/google/cloud/sql/connector/exceptions.py index da39ea25d..1f15ced47 100644 --- a/google/cloud/sql/connector/exceptions.py +++ b/google/cloud/sql/connector/exceptions.py @@ -84,3 +84,10 @@ class CacheClosedError(Exception): Exception to be raised when a ConnectionInfoCache can not be accessed after it is closed. """ + + +class ClosedConnectorError(Exception): + """ + Exception to be raised when a Connector is closed and connect method is + called on it. + """ diff --git a/tests/unit/test_connector.py b/tests/unit/test_connector.py index cd4746d3e..8889bafa2 100644 --- a/tests/unit/test_connector.py +++ b/tests/unit/test_connector.py @@ -16,7 +16,6 @@ import asyncio import os -import time from typing import Union from aiohttp import ClientResponseError @@ -31,6 +30,7 @@ from google.cloud.sql.connector.connection_name import ConnectionName from google.cloud.sql.connector.exceptions import CloudSQLIPTypeError from google.cloud.sql.connector.exceptions import IncompatibleDriverError +from google.cloud.sql.connector.exceptions import ClosedConnectionError from google.cloud.sql.connector.instance import RefreshAheadCache @@ -481,9 +481,7 @@ async def test_connect_async_closed_connector( ) as connector: connector._client = fake_client await connector.close_async() - # wait for close to complete - # await asyncio.sleep(0.1) - with pytest.raises(RuntimeError) as exc_info: + with pytest.raises(ClosedConnectionError) as exc_info: await connector.connect_async( "test-project:test-region:test-instance", "asyncpg", @@ -491,7 +489,10 @@ async def test_connect_async_closed_connector( password="my-pass", db="my-db", ) - assert exc_info.value.args[0] == "Cannot connect using a closed Connector." + assert ( + exc_info.value.args[0] + == "Connection attempt failed because the connector has already been closed." + ) def test_connect_closed_connector( @@ -501,8 +502,7 @@ def test_connect_closed_connector( with Connector(credentials=fake_credentials) as connector: connector._client = fake_client connector.close() - # time.sleep(3.1) - with pytest.raises(RuntimeError) as exc_info: + with pytest.raises(ClosedConnectionError) as exc_info: connector.connect( "test-project:test-region:test-instance", "pg8000", @@ -510,4 +510,7 @@ def test_connect_closed_connector( password="my-pass", db="my-db", ) - assert exc_info.value.args[0] == "Cannot connect using a closed Connector." + assert ( + exc_info.value.args[0] + == "Connection attempt failed because the connector has already been closed." + ) From b8bc9120bbc6005c55decbadffd3c8d0a0beb100 Mon Sep 17 00:00:00 2001 From: kgala2 Date: Fri, 16 May 2025 15:31:02 -0700 Subject: [PATCH 8/9] chore: update import statements --- google/cloud/sql/connector/connector.py | 2 +- tests/unit/test_connector.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/google/cloud/sql/connector/connector.py b/google/cloud/sql/connector/connector.py index a674f25b6..df519cd41 100755 --- a/google/cloud/sql/connector/connector.py +++ b/google/cloud/sql/connector/connector.py @@ -33,6 +33,7 @@ from google.cloud.sql.connector.enums import DriverMapping from google.cloud.sql.connector.enums import IPTypes from google.cloud.sql.connector.enums import RefreshStrategy +from google.cloud.sql.connector.exceptions import ClosedConnectorError from google.cloud.sql.connector.instance import RefreshAheadCache from google.cloud.sql.connector.lazy import LazyRefreshCache from google.cloud.sql.connector.monitored_cache import MonitoredCache @@ -43,7 +44,6 @@ from google.cloud.sql.connector.resolver import DnsResolver from google.cloud.sql.connector.utils import format_database_user from google.cloud.sql.connector.utils import generate_keys -from google.cloud.sql.connector.exceptions import ClosedConnectorError logger = logging.getLogger(name=__name__) diff --git a/tests/unit/test_connector.py b/tests/unit/test_connector.py index 8889bafa2..e943eae0d 100644 --- a/tests/unit/test_connector.py +++ b/tests/unit/test_connector.py @@ -29,8 +29,8 @@ from google.cloud.sql.connector.client import CloudSQLClient from google.cloud.sql.connector.connection_name import ConnectionName from google.cloud.sql.connector.exceptions import CloudSQLIPTypeError +from google.cloud.sql.connector.exceptions import ClosedConnectorError from google.cloud.sql.connector.exceptions import IncompatibleDriverError -from google.cloud.sql.connector.exceptions import ClosedConnectionError from google.cloud.sql.connector.instance import RefreshAheadCache @@ -481,7 +481,7 @@ async def test_connect_async_closed_connector( ) as connector: connector._client = fake_client await connector.close_async() - with pytest.raises(ClosedConnectionError) as exc_info: + with pytest.raises(ClosedConnectorError) as exc_info: await connector.connect_async( "test-project:test-region:test-instance", "asyncpg", @@ -502,7 +502,7 @@ def test_connect_closed_connector( with Connector(credentials=fake_credentials) as connector: connector._client = fake_client connector.close() - with pytest.raises(ClosedConnectionError) as exc_info: + with pytest.raises(ClosedConnectorError) as exc_info: connector.connect( "test-project:test-region:test-instance", "pg8000", From e1f3d4ff076c90eef89ba5c9aadc4cd8f8b15288 Mon Sep 17 00:00:00 2001 From: kgala2 Date: Fri, 16 May 2025 16:15:13 -0700 Subject: [PATCH 9/9] chore: reorder imports in test file --- tests/unit/test_connector.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_connector.py b/tests/unit/test_connector.py index e943eae0d..bde7f65a9 100644 --- a/tests/unit/test_connector.py +++ b/tests/unit/test_connector.py @@ -28,8 +28,8 @@ from google.cloud.sql.connector import IPTypes from google.cloud.sql.connector.client import CloudSQLClient from google.cloud.sql.connector.connection_name import ConnectionName -from google.cloud.sql.connector.exceptions import CloudSQLIPTypeError from google.cloud.sql.connector.exceptions import ClosedConnectorError +from google.cloud.sql.connector.exceptions import CloudSQLIPTypeError from google.cloud.sql.connector.exceptions import IncompatibleDriverError from google.cloud.sql.connector.instance import RefreshAheadCache