Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spanner: add client_info support to client. #7878

Merged
merged 4 commits into from
May 8, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 27 additions & 12 deletions spanner/google/cloud/spanner_v1/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
* a :class:`~google.cloud.spanner_v1.instance.Instance` owns a
:class:`~google.cloud.spanner_v1.database.Database`
"""
import warnings

from google.api_core.gapic_v1 import client_info

Expand All @@ -36,7 +37,6 @@

# pylint: enable=line-too-long

from google.cloud._http import DEFAULT_USER_AGENT
from google.cloud.client import ClientWithProject
from google.cloud.spanner_v1 import __version__
from google.cloud.spanner_v1._helpers import _metadata_with_prefix
Expand All @@ -45,6 +45,10 @@

_CLIENT_INFO = client_info.ClientInfo(client_library_version=__version__)
SPANNER_ADMIN_SCOPE = "https://www.googleapis.com/auth/spanner.admin"
_USER_AGENT_DEPRECATED = (
"The 'user_agent' argument to 'Client' is deprecated / unused. "
"Please pass an appropriate 'client_info' instead."
)


class InstanceConfig(object):
Expand Down Expand Up @@ -95,29 +99,44 @@ class Client(ClientWithProject):
client. If not provided, defaults to the Google
Application Default Credentials.

:type client_info: :class:`google.api_core.gapic_v1.client_info.ClientInfo`
:param client_info:
(Optional) The client info used to send a user-agent string along with
API requests. If ``None``, then default info will be used. Generally,
you only need to set this if you're developing your own library or
partner tool.

:type user_agent: str
:param user_agent: (Optional) The user agent to be used with API request.
Defaults to :const:`DEFAULT_USER_AGENT`.
:param user_agent:
(Deprecated) The user agent to be used with API request.
Not used.

:raises: :class:`ValueError <exceptions.ValueError>` if both ``read_only``
and ``admin`` are :data:`True`
"""

_instance_admin_api = None
_database_admin_api = None
user_agent = None
_SET_PROJECT = True # Used by from_service_account_json()

SCOPE = (SPANNER_ADMIN_SCOPE,)
"""The scopes required for Google Cloud Spanner."""

def __init__(self, project=None, credentials=None, user_agent=DEFAULT_USER_AGENT):
def __init__(
self, project=None, credentials=None, client_info=_CLIENT_INFO, user_agent=None
):
# NOTE: This API has no use for the _http argument, but sending it
# will have no impact since the _http() @property only lazily
# creates a working HTTP object.
super(Client, self).__init__(
project=project, credentials=credentials, _http=None
)
self.user_agent = user_agent
self._client_info = client_info

if user_agent is not None:
warnings.warn(_USER_AGENT_DEPRECATED, DeprecationWarning, stacklevel=2)
self.user_agent = user_agent

@property
def credentials(self):
Expand Down Expand Up @@ -153,7 +172,7 @@ def instance_admin_api(self):
"""Helper for session-related API calls."""
if self._instance_admin_api is None:
self._instance_admin_api = InstanceAdminClient(
credentials=self.credentials, client_info=_CLIENT_INFO
credentials=self.credentials, client_info=self._client_info
)
return self._instance_admin_api

Expand All @@ -162,7 +181,7 @@ def database_admin_api(self):
"""Helper for session-related API calls."""
if self._database_admin_api is None:
self._database_admin_api = DatabaseAdminClient(
credentials=self.credentials, client_info=_CLIENT_INFO
credentials=self.credentials, client_info=self._client_info
)
return self._database_admin_api

Expand All @@ -175,11 +194,7 @@ def copy(self):
:rtype: :class:`.Client`
:returns: A copy of the current client.
"""
return self.__class__(
project=self.project,
credentials=self._credentials,
user_agent=self.user_agent,
)
return self.__class__(project=self.project, credentials=self._credentials)

def list_instance_configs(self, page_size=None, page_token=None):
"""List available instance configurations for the client's project.
Expand Down
6 changes: 2 additions & 4 deletions spanner/google/cloud/spanner_v1/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,12 @@
import re
import threading

from google.api_core.gapic_v1 import client_info
import google.auth.credentials
from google.protobuf.struct_pb2 import Struct
from google.cloud.exceptions import NotFound
import six

# pylint: disable=ungrouped-imports
from google.cloud.spanner_v1 import __version__
from google.cloud.spanner_v1._helpers import _make_value_pb
from google.cloud.spanner_v1._helpers import _metadata_with_prefix
from google.cloud.spanner_v1.batch import Batch
Expand All @@ -46,7 +44,6 @@
# pylint: enable=ungrouped-imports


_CLIENT_INFO = client_info.ClientInfo(client_library_version=__version__)
SPANNER_DATA_SCOPE = "https://www.googleapis.com/auth/spanner.data"


Expand Down Expand Up @@ -179,8 +176,9 @@ def spanner_api(self):
credentials = self._instance._client.credentials
if isinstance(credentials, google.auth.credentials.Scoped):
credentials = credentials.with_scopes((SPANNER_DATA_SCOPE,))
client_info = self._instance._client._client_info
self._spanner_api = SpannerClient(
credentials=credentials, client_info=_CLIENT_INFO
credentials=credentials, client_info=client_info
)
return self._spanner_api

Expand Down
55 changes: 41 additions & 14 deletions spanner/tests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,24 @@ def _make_one(self, *args, **kwargs):
return self._get_target_class()(*args, **kwargs)

def _constructor_test_helper(
self, expected_scopes, creds, user_agent=None, expected_creds=None
self,
expected_scopes,
creds,
expected_creds=None,
client_info=None,
user_agent=None,
):
from google.cloud.spanner_v1 import client as MUT

user_agent = user_agent or MUT.DEFAULT_USER_AGENT
kwargs = {}

if client_info is not None:
kwargs["client_info"] = expected_client_info = client_info
else:
expected_client_info = MUT._CLIENT_INFO

client = self._make_one(
project=self.PROJECT, credentials=creds, user_agent=user_agent
project=self.PROJECT, credentials=creds, user_agent=user_agent, **kwargs
)

expected_creds = expected_creds or creds.with_scopes.return_value
Expand All @@ -66,6 +77,7 @@ def _constructor_test_helper(
creds.with_scopes.assert_called_once_with(expected_scopes)

self.assertEqual(client.project, self.PROJECT)
self.assertIs(client._client_info, expected_client_info)
self.assertEqual(client.user_agent, user_agent)

def test_constructor_default_scopes(self):
Expand All @@ -75,7 +87,8 @@ def test_constructor_default_scopes(self):
creds = _make_credentials()
self._constructor_test_helper(expected_scopes, creds)

def test_constructor_custom_user_agent_and_timeout(self):
@mock.patch("warnings.warn")
def test_constructor_custom_user_agent_and_timeout(self, mock_warn):
from google.cloud.spanner_v1 import client as MUT

CUSTOM_USER_AGENT = "custom-application"
Expand All @@ -84,6 +97,17 @@ def test_constructor_custom_user_agent_and_timeout(self):
self._constructor_test_helper(
expected_scopes, creds, user_agent=CUSTOM_USER_AGENT
)
mock_warn.assert_called_once_with(
MUT._USER_AGENT_DEPRECATED, DeprecationWarning, stacklevel=2
)

def test_constructor_custom_client_info(self):
from google.cloud.spanner_v1 import client as MUT

client_info = mock.Mock()
expected_scopes = (MUT.SPANNER_ADMIN_SCOPE,)
creds = _make_credentials()
self._constructor_test_helper(expected_scopes, creds, client_info=client_info)

def test_constructor_implicit_credentials(self):
creds = _make_credentials()
Expand All @@ -102,10 +126,13 @@ def test_constructor_credentials_wo_create_scoped(self):
self._constructor_test_helper(expected_scopes, creds)

def test_instance_admin_api(self):
from google.cloud.spanner_v1.client import _CLIENT_INFO, SPANNER_ADMIN_SCOPE
from google.cloud.spanner_v1.client import SPANNER_ADMIN_SCOPE

credentials = _make_credentials()
client = self._make_one(project=self.PROJECT, credentials=credentials)
client_info = mock.Mock()
client = self._make_one(
project=self.PROJECT, credentials=credentials, client_info=client_info
)
expected_scopes = (SPANNER_ADMIN_SCOPE,)

inst_module = "google.cloud.spanner_v1.client.InstanceAdminClient"
Expand All @@ -119,16 +146,19 @@ def test_instance_admin_api(self):
self.assertIs(again, api)

instance_admin_client.assert_called_once_with(
credentials=credentials.with_scopes.return_value, client_info=_CLIENT_INFO
credentials=credentials.with_scopes.return_value, client_info=client_info
)

credentials.with_scopes.assert_called_once_with(expected_scopes)

def test_database_admin_api(self):
from google.cloud.spanner_v1.client import _CLIENT_INFO, SPANNER_ADMIN_SCOPE
from google.cloud.spanner_v1.client import SPANNER_ADMIN_SCOPE

credentials = _make_credentials()
client = self._make_one(project=self.PROJECT, credentials=credentials)
client_info = mock.Mock()
client = self._make_one(
project=self.PROJECT, credentials=credentials, client_info=client_info
)
expected_scopes = (SPANNER_ADMIN_SCOPE,)

db_module = "google.cloud.spanner_v1.client.DatabaseAdminClient"
Expand All @@ -142,7 +172,7 @@ def test_database_admin_api(self):
self.assertIs(again, api)

database_admin_client.assert_called_once_with(
credentials=credentials.with_scopes.return_value, client_info=_CLIENT_INFO
credentials=credentials.with_scopes.return_value, client_info=client_info
)

credentials.with_scopes.assert_called_once_with(expected_scopes)
Expand All @@ -152,14 +182,11 @@ def test_copy(self):
# Make sure it "already" is scoped.
credentials.requires_scopes = False

client = self._make_one(
project=self.PROJECT, credentials=credentials, user_agent=self.USER_AGENT
)
client = self._make_one(project=self.PROJECT, credentials=credentials)

new_client = client.copy()
self.assertIs(new_client._credentials, client._credentials)
self.assertEqual(new_client.project, client.project)
self.assertEqual(new_client.user_agent, client.user_agent)

def test_credentials_property(self):
credentials = _make_credentials()
Expand Down
10 changes: 5 additions & 5 deletions spanner/tests/unit/test_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,8 @@ def test_name_property(self):
self.assertEqual(database.name, expected_name)

def test_spanner_api_property_w_scopeless_creds(self):
from google.cloud.spanner_v1.database import _CLIENT_INFO

client = _Client()
client_info = client._client_info = mock.Mock()
credentials = client.credentials = object()
instance = _Instance(self.INSTANCE_NAME, client=client)
pool = _Pool()
Expand All @@ -251,12 +250,12 @@ def test_spanner_api_property_w_scopeless_creds(self):
self.assertIs(again, api)

spanner_client.assert_called_once_with(
credentials=credentials, client_info=_CLIENT_INFO
credentials=credentials, client_info=client_info
)

def test_spanner_api_w_scoped_creds(self):
import google.auth.credentials
from google.cloud.spanner_v1.database import _CLIENT_INFO, SPANNER_DATA_SCOPE
from google.cloud.spanner_v1.database import SPANNER_DATA_SCOPE

class _CredentialsWithScopes(google.auth.credentials.Scoped):
def __init__(self, scopes=(), source=None):
Expand All @@ -271,6 +270,7 @@ def with_scopes(self, scopes):

expected_scopes = (SPANNER_DATA_SCOPE,)
client = _Client()
client_info = client._client_info = mock.Mock()
credentials = client.credentials = _CredentialsWithScopes()
instance = _Instance(self.INSTANCE_NAME, client=client)
pool = _Pool()
Expand All @@ -290,7 +290,7 @@ def with_scopes(self, scopes):
self.assertEqual(len(spanner_client.call_args_list), 1)
called_args, called_kw = spanner_client.call_args
self.assertEqual(called_args, ())
self.assertEqual(called_kw["client_info"], _CLIENT_INFO)
self.assertEqual(called_kw["client_info"], client_info)
scoped = called_kw["credentials"]
self.assertEqual(scoped._scopes, expected_scopes)
self.assertIs(scoped._source, credentials)
Expand Down