From 45ad56e1d5b34d94998767c0e117eddd974e131b Mon Sep 17 00:00:00 2001 From: HemangChothani Date: Tue, 2 Feb 2021 17:44:18 +0530 Subject: [PATCH 1/3] fix: credentials_uri parameter error --- google/cloud/spanner_dbapi/connection.py | 17 +++++++++++++--- tests/unit/spanner_dbapi/test_connect.py | 26 ++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/google/cloud/spanner_dbapi/connection.py b/google/cloud/spanner_dbapi/connection.py index 6438605d3b..b3230a0136 100644 --- a/google/cloud/spanner_dbapi/connection.py +++ b/google/cloud/spanner_dbapi/connection.py @@ -339,6 +339,7 @@ def connect( credentials=None, pool=None, user_agent=None, + credentials_uri=None, ): """Creates a connection to a Google Cloud Spanner database. @@ -368,6 +369,11 @@ def connect( :param user_agent: (Optional) User agent to be used with this connection's requests. + :type credentials_uri: str + :param credentials_uri: (Optional) An optional string specifying where to retrieve + the service account JSON for the credentials to connect to + Cloud Spanner. + :rtype: :class:`google.cloud.spanner_dbapi.connection.Connection` :returns: Connection object associated with the given Google Cloud Spanner resource. @@ -380,9 +386,14 @@ def connect( user_agent=user_agent or DEFAULT_USER_AGENT, python_version=PY_VERSION ) - client = spanner.Client( - project=project, credentials=credentials, client_info=client_info - ) + if credentials_uri: + client = spanner.Client.from_service_account_json( + credentials_uri, project=project, client_info=client_info + ) + else: + client = spanner.Client( + project=project, credentials=credentials, client_info=client_info + ) instance = client.instance(instance_id) if not instance.exists(): diff --git a/tests/unit/spanner_dbapi/test_connect.py b/tests/unit/spanner_dbapi/test_connect.py index 771b9d4a7f..ffa7018364 100644 --- a/tests/unit/spanner_dbapi/test_connect.py +++ b/tests/unit/spanner_dbapi/test_connect.py @@ -139,3 +139,29 @@ def test_sessions_pool(self): ): connect("test-instance", database_id, pool=pool) database_mock.assert_called_once_with(database_id, pool=pool) + + def test_connect_w_coonection_uri(self): + from google.cloud.spanner_dbapi import connect + from google.cloud.spanner_dbapi import Connection + + PROJECT = "test-project" + USER_AGENT = "user-agent" + credentials_uri = "dummy/file/path.json" + + with mock.patch( + "google.cloud.spanner_v1.Client.from_service_account_json" + ) as client_mock: + connection = connect( + "test-instance", + "test-database", + PROJECT, + None, + user_agent=USER_AGENT, + credentials_uri=credentials_uri, + ) + + self.assertIsInstance(connection, Connection) + + client_mock.assert_called_once_with( + credentials_uri, project=PROJECT, client_info=mock.ANY + ) From b830c6566e2183a2443af4554831d2b2d36258e4 Mon Sep 17 00:00:00 2001 From: HemangChothani Date: Wed, 10 Feb 2021 18:32:35 +0530 Subject: [PATCH 2/3] fix: combine credentials and credentials_uri --- google/cloud/spanner_dbapi/connection.py | 14 +++++--------- tests/unit/spanner_dbapi/test_connect.py | 9 ++++----- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/google/cloud/spanner_dbapi/connection.py b/google/cloud/spanner_dbapi/connection.py index b3230a0136..fdbfa200d5 100644 --- a/google/cloud/spanner_dbapi/connection.py +++ b/google/cloud/spanner_dbapi/connection.py @@ -339,7 +339,6 @@ def connect( credentials=None, pool=None, user_agent=None, - credentials_uri=None, ): """Creates a connection to a Google Cloud Spanner database. @@ -357,7 +356,9 @@ def connect( :type credentials: :class:`~google.auth.credentials.Credentials` :param credentials: (Optional) The authorization credentials to attach to requests. These credentials identify this application - to the service. If none are specified, the client will + to the service. Or a string specifying (path) where to retrieve + the service account JSON for the credentials to connect to + Cloud Spanner.If none are specified, the client will attempt to ascertain the credentials from the environment. @@ -369,11 +370,6 @@ def connect( :param user_agent: (Optional) User agent to be used with this connection's requests. - :type credentials_uri: str - :param credentials_uri: (Optional) An optional string specifying where to retrieve - the service account JSON for the credentials to connect to - Cloud Spanner. - :rtype: :class:`google.cloud.spanner_dbapi.connection.Connection` :returns: Connection object associated with the given Google Cloud Spanner resource. @@ -386,9 +382,9 @@ def connect( user_agent=user_agent or DEFAULT_USER_AGENT, python_version=PY_VERSION ) - if credentials_uri: + if isinstance(credentials, str): client = spanner.Client.from_service_account_json( - credentials_uri, project=project, client_info=client_info + credentials, project=project, client_info=client_info ) else: client = spanner.Client( diff --git a/tests/unit/spanner_dbapi/test_connect.py b/tests/unit/spanner_dbapi/test_connect.py index ffa7018364..a18781ffd1 100644 --- a/tests/unit/spanner_dbapi/test_connect.py +++ b/tests/unit/spanner_dbapi/test_connect.py @@ -140,13 +140,13 @@ def test_sessions_pool(self): connect("test-instance", database_id, pool=pool) database_mock.assert_called_once_with(database_id, pool=pool) - def test_connect_w_coonection_uri(self): + def test_connect_w_credential_file_path(self): from google.cloud.spanner_dbapi import connect from google.cloud.spanner_dbapi import Connection PROJECT = "test-project" USER_AGENT = "user-agent" - credentials_uri = "dummy/file/path.json" + credentials = "dummy/file/path.json" with mock.patch( "google.cloud.spanner_v1.Client.from_service_account_json" @@ -155,13 +155,12 @@ def test_connect_w_coonection_uri(self): "test-instance", "test-database", PROJECT, - None, + credentials=credentials, user_agent=USER_AGENT, - credentials_uri=credentials_uri, ) self.assertIsInstance(connection, Connection) client_mock.assert_called_once_with( - credentials_uri, project=PROJECT, client_info=mock.ANY + credentials, project=PROJECT, client_info=mock.ANY ) From 2044ce82fd9fa3e8a6ad777670bfd9c8e9006f8b Mon Sep 17 00:00:00 2001 From: HemangChothani Date: Mon, 1 Mar 2021 11:12:15 +0530 Subject: [PATCH 3/3] fix: nits --- google/cloud/spanner_dbapi/connection.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/google/cloud/spanner_dbapi/connection.py b/google/cloud/spanner_dbapi/connection.py index fdbfa200d5..5973b559d6 100644 --- a/google/cloud/spanner_dbapi/connection.py +++ b/google/cloud/spanner_dbapi/connection.py @@ -353,12 +353,13 @@ def connect( instances, tables and data. If not provided, will attempt to determine from the environment. - :type credentials: :class:`~google.auth.credentials.Credentials` + :type credentials: Union[:class:`~google.auth.credentials.Credentials`, str] :param credentials: (Optional) The authorization credentials to attach to requests. These credentials identify this application - to the service. Or a string specifying (path) where to retrieve - the service account JSON for the credentials to connect to - Cloud Spanner.If none are specified, the client will + to the service. These credentials may be specified as + a file path indicating where to retrieve the service + account JSON for the credentials to connect to + Cloud Spanner. If none are specified, the client will attempt to ascertain the credentials from the environment.