fix!: update error messages and doc strings (GoogleCloudPlatform#276)

Author: jackwotherspoon

google/cloud/sql/connector/__init__.py

@@ -17,10 +17,10 @@
 from typing import List
 
 from .connector import connect
-from .instance_connection_manager import CloudSQLConnectionError, IPTypes
+from .instance_connection_manager import IPTypes
 
 
-__ALL__ = [connect, CloudSQLConnectionError, IPTypes]
+__ALL__ = [connect, IPTypes]
 
 try:
     import pkg_resources

google/cloud/sql/connector/connector.py

@@ -113,11 +113,10 @@ def connect(
             icm = self._instances[instance_connection_string]
             if enable_iam_auth != icm._enable_iam_auth:
                 raise ValueError(
-                    "connect() called with `enable_iam_auth={}`, but previously used "
-                    "enable_iam_auth={}`. If you require both for your use case, "
-                    "please use a new connector.Connector object.".format(
-                        enable_iam_auth, icm._enable_iam_auth
-                    )
+                    f"connect() called with `enable_iam_auth={enable_iam_auth}`, "
+                    f"but previously used enable_iam_auth={icm._enable_iam_auth}`. "
+                    "If you require both for your use case, please use a new "
+                    "connector.Connector object."
                 )
         else:
             icm = InstanceConnectionManager(

google/cloud/sql/connector/instance_connection_manager.py

@@ -89,16 +89,6 @@ def __init__(self, *args: Any) -> None:
         super(TLSVersionError, self).__init__(self, *args)
 
 
-class CloudSQLConnectionError(Exception):
-    """
-    Raised when the provided connection string is not formatted
-    correctly.
-    """
-
-    def __init__(self, *args: Any) -> None:
-        super(CloudSQLConnectionError, self).__init__(self, *args)
-
-
 class CloudSQLIPTypeError(Exception):
     """
     Raised when IP address for the preferred IP type is not found.
@@ -256,9 +246,10 @@ def __init__(
             self._region = connection_string_split[1]
             self._instance = connection_string_split[2]
         else:
-            raise CloudSQLConnectionError(
-                "Arg instance_connection_string must be in "
-                + "format: project:region:instance."
+            raise ValueError(
+                "Arg `instance_connection_string` must have "
+                "format: PROJECT:REGION:INSTANCE, "
+                f"got {instance_connection_string}."
             )
 
         self._enable_iam_auth = enable_iam_auth
@@ -566,7 +557,7 @@ async def _connect(
         try:
             connector = connect_func[driver]
         except KeyError:
-            raise KeyError("Driver {} is not supported.".format(driver))
+            raise KeyError(f"Driver {driver} is not supported.")
 
         connect_partial = partial(
             connector, ip_address, instance_data.context, **kwargs

google/cloud/sql/connector/refresh_utils.py

@@ -39,7 +39,7 @@ async def _get_metadata(
     and returns a dictionary containing the IP addresses and certificate
     authority of the Cloud SQL Instance.
 
-    :type credentials: google.oauth2.service_account.Credentials
+    :type credentials: google.auth.credentials.Credentials
     :param credentials:
         A credentials object created from the google-auth Python library.
         Must have the SQL Admin API scopes. For more info check out
@@ -60,16 +60,15 @@ async def _get_metadata(
     :raises TypeError: If any of the arguments are not the specified type.
     """
 
-    if (
-        not isinstance(credentials, Credentials)
-        or not isinstance(project, str)
-        or not isinstance(instance, str)
-    ):
+    if not isinstance(credentials, Credentials):
         raise TypeError(
-            "Arguments must be as follows: "
-            + "credentials (google.oauth2.service_account.Credentials), "
-            + "project (str) and instance (str)."
+            "credentials must be of type google.auth.credentials.Credentials,"
+            f" got {type(credentials)}"
         )
+    elif not isinstance(project, str):
+        raise TypeError(f"project must be of type str, got {type(project)}")
+    elif not isinstance(instance, str):
+        raise TypeError(f"instance must be of type str, got {type(instance)}")
 
     if not credentials.valid:
         request = google.auth.transport.requests.Request()
@@ -106,7 +105,7 @@ async def _get_ephemeral(
 ) -> str:
     """Asynchronously requests an ephemeral certificate from the Cloud SQL Instance.
 
-    :type credentials: google.oauth2.service_account.Credentials
+    :type credentials: google.auth.credentials.Credentials
     :param credentials: A credentials object
         created from the google-auth library. Must be
         using the SQL Admin API scopes. For more info, check out
@@ -136,8 +135,8 @@ async def _get_ephemeral(
 
     if not isinstance(credentials, Credentials):
         raise TypeError(
-            "credentials must be of type google.oauth2.service_account.Credentials,"
-            f"got {type(credentials)}"
+            "credentials must be of type google.auth.credentials.Credentials,"
+            f" got {type(credentials)}"
         )
     elif not isinstance(project, str):
         raise TypeError(f"project must be of type str, got {type(project)}")

tests/unit/test_refresh_utils.py

@@ -13,7 +13,7 @@
 See the License for the specific language governing permissions and
 limitations under the License.
 """
-from typing import Any
+from typing import Any, no_type_check
 
 import aiohttp
 from google.auth.credentials import Credentials
@@ -101,6 +101,56 @@ async def test_get_ephemeral(mock_post: AsyncMock, credentials: Credentials) ->
     )
 
 
+@pytest.mark.asyncio
+@no_type_check
+async def test_get_ephemeral_TypeError(credentials: Credentials) -> None:
+    """
+    Test to check whether _get_ephemeral throws proper TypeError
+    when given incorrect input arg types.
+    """
+    client_session = Mock(aiohttp.ClientSession)
+    project = "my-project"
+    instance = "my-instance"
+    pub_key = "key"
+
+    # incorrect credentials type
+    with pytest.raises(TypeError):
+        await _get_ephemeral(
+            client_session=client_session,
+            credentials="bad-credentials",
+            project=project,
+            instance=instance,
+            pub_key=pub_key,
+        )
+    # incorrect project type
+    with pytest.raises(TypeError):
+        await _get_ephemeral(
+            client_session=client_session,
+            credentials=credentials,
+            project=12345,
+            instance=instance,
+            pub_key=pub_key,
+        )
+    # incorrect instance type
+    with pytest.raises(TypeError):
+        await _get_ephemeral(
+            client_session=client_session,
+            credentials=credentials,
+            project=project,
+            instance=12345,
+            pub_key=pub_key,
+        )
+    # incorrect pub_key type
+    with pytest.raises(TypeError):
+        await _get_ephemeral(
+            client_session=client_session,
+            credentials=credentials,
+            project=project,
+            instance=instance,
+            pub_key=12345,
+        )
+
+
 @pytest.mark.asyncio
 @patch("aiohttp.ClientSession.get", new_callable=AsyncMock)
 async def test_get_metadata(mock_get: AsyncMock, credentials: Credentials) -> None:
@@ -119,3 +169,40 @@ async def test_get_metadata(mock_get: AsyncMock, credentials: Credentials) -> No
     assert result["ip_addresses"] is not None and isinstance(
         result["server_ca_cert"], str
     )
+
+
+@pytest.mark.asyncio
+@no_type_check
+async def test_get_metadata_TypeError(credentials: Credentials) -> None:
+    """
+    Test to check whether _get_metadata throws proper TypeError
+    when given incorrect input arg types.
+    """
+    client_session = Mock(aiohttp.ClientSession)
+    project = "my-project"
+    instance = "my-instance"
+
+    # incorrect credentials type
+    with pytest.raises(TypeError):
+        await _get_metadata(
+            client_session=client_session,
+            credentials="bad-credentials",
+            project=project,
+            instance=instance,
+        )
+    # incorrect project type
+    with pytest.raises(TypeError):
+        await _get_metadata(
+            client_session=client_session,
+            credentials=credentials,
+            project=12345,
+            instance=instance,
+        )
+    # incorrect instance type
+    with pytest.raises(TypeError):
+        await _get_metadata(
+            client_session=client_session,
+            credentials=credentials,
+            project=project,
+            instance=12345,
+        )