diff --git a/datastore/google/cloud/datastore/_gapic.py b/datastore/google/cloud/datastore/_gapic.py index 1210f2821802..18c2ce917bc2 100644 --- a/datastore/google/cloud/datastore/_gapic.py +++ b/datastore/google/cloud/datastore/_gapic.py @@ -14,15 +14,12 @@ """Helpers for making API requests via gapic / gRPC.""" -from google.api_core.gapic_v1 import client_info -from google.cloud.datastore_v1.gapic import datastore_client from grpc import insecure_channel import six from google.cloud._helpers import make_secure_channel from google.cloud._http import DEFAULT_USER_AGENT - -from google.cloud.datastore import __version__ +from google.cloud.datastore_v1.gapic import datastore_client def make_datastore_api(client): @@ -42,8 +39,5 @@ def make_datastore_api(client): channel = insecure_channel(host) return datastore_client.DatastoreClient( - channel=channel, - client_info=client_info.ClientInfo( - client_library_version=__version__, gapic_version=__version__ - ), + channel=channel, client_info=client._client_info ) diff --git a/datastore/google/cloud/datastore/_http.py b/datastore/google/cloud/datastore/_http.py index 03a551cec64e..0d10035340c6 100644 --- a/datastore/google/cloud/datastore/_http.py +++ b/datastore/google/cloud/datastore/_http.py @@ -20,8 +20,6 @@ from google.cloud import exceptions from google.cloud.datastore_v1.proto import datastore_pb2 as _datastore_pb2 -from google.cloud.datastore import __version__ - DATASTORE_API_HOST = "datastore.googleapis.com" """Datastore API request host.""" @@ -32,10 +30,8 @@ API_URL_TEMPLATE = "{api_base}/{api_version}/projects" "/{project}:{method}" """A template for the URL of a particular API call.""" -_CLIENT_INFO = connection_module.CLIENT_INFO_TEMPLATE.format(__version__) - -def _request(http, project, method, data, base_url): +def _request(http, project, method, data, base_url, client_info): """Make a request over the Http transport to the Cloud Datastore API. :type http: :class:`requests.Session` @@ -55,15 +51,19 @@ def _request(http, project, method, data, base_url): :type base_url: str :param base_url: The base URL where the API lives. + :type client_info: :class:`google.api_core.client_info.ClientInfo` + :param client_info: used to generate user agent. + :rtype: str :returns: The string response content from the API call. :raises: :class:`google.cloud.exceptions.GoogleCloudError` if the response code is not 200 OK. """ + user_agent = client_info.to_user_agent() headers = { "Content-Type": "application/x-protobuf", - "User-Agent": connection_module.DEFAULT_USER_AGENT, - connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO, + "User-Agent": user_agent, + connection_module.CLIENT_INFO_HEADER: user_agent, } api_url = build_api_url(project, method, base_url) @@ -78,7 +78,7 @@ def _request(http, project, method, data, base_url): return response.content -def _rpc(http, project, method, base_url, request_pb, response_pb_cls): +def _rpc(http, project, method, base_url, client_info, request_pb, response_pb_cls): """Make a protobuf RPC request. :type http: :class:`requests.Session` @@ -94,6 +94,9 @@ def _rpc(http, project, method, base_url, request_pb, response_pb_cls): :type base_url: str :param base_url: The base URL where the API lives. + :type client_info: :class:`google.api_core.client_info.ClientInfo` + :param client_info: used to generate user agent. + :type request_pb: :class:`google.protobuf.message.Message` instance :param request_pb: the protobuf instance representing the request. @@ -106,7 +109,7 @@ def _rpc(http, project, method, base_url, request_pb, response_pb_cls): :returns: The RPC message parsed from the response. """ req_data = request_pb.SerializeToString() - response = _request(http, project, method, req_data, base_url) + response = _request(http, project, method, req_data, base_url, client_info) return response_pb_cls.FromString(response) @@ -172,6 +175,7 @@ def lookup(self, project_id, keys, read_options=None): project_id, "lookup", self.client._base_url, + self.client._client_info, request_pb, _datastore_pb2.LookupResponse, ) @@ -217,6 +221,7 @@ def run_query( project_id, "runQuery", self.client._base_url, + self.client._client_info, request_pb, _datastore_pb2.RunQueryResponse, ) @@ -240,6 +245,7 @@ def begin_transaction(self, project_id, transaction_options=None): project_id, "beginTransaction", self.client._base_url, + self.client._client_info, request_pb, _datastore_pb2.BeginTransactionResponse, ) @@ -278,6 +284,7 @@ def commit(self, project_id, mode, mutations, transaction=None): project_id, "commit", self.client._base_url, + self.client._client_info, request_pb, _datastore_pb2.CommitResponse, ) @@ -304,6 +311,7 @@ def rollback(self, project_id, transaction): project_id, "rollback", self.client._base_url, + self.client._client_info, request_pb, _datastore_pb2.RollbackResponse, ) @@ -327,6 +335,7 @@ def allocate_ids(self, project_id, keys): project_id, "allocateIds", self.client._base_url, + self.client._client_info, request_pb, _datastore_pb2.AllocateIdsResponse, ) diff --git a/datastore/google/cloud/datastore/client.py b/datastore/google/cloud/datastore/client.py index 6f4d82eae672..9cf892aab54d 100644 --- a/datastore/google/cloud/datastore/client.py +++ b/datastore/google/cloud/datastore/client.py @@ -18,6 +18,7 @@ from google.cloud._helpers import _LocalStack from google.cloud._helpers import _determine_default_project as _base_default_project from google.cloud.client import ClientWithProject +from google.cloud.datastore import __version__ from google.cloud.datastore import helpers from google.cloud.datastore._http import HTTPDatastoreAPI from google.cloud.datastore.batch import Batch @@ -32,10 +33,19 @@ try: from google.cloud.datastore._gapic import make_datastore_api - _HAVE_GRPC = True except ImportError: # pragma: NO COVER + from google.api_core import client_info + make_datastore_api = None _HAVE_GRPC = False + _CLIENT_INFO = client_info.ClientInfo(client_library_version=__version__) +else: + from google.api_core.gapic_v1 import client_info + + _HAVE_GRPC = True + _CLIENT_INFO = client_info.ClientInfo( + client_library_version=__version__, gapic_version=__version__ + ) _MAX_LOOPS = 128 @@ -43,6 +53,7 @@ _DATASTORE_BASE_URL = "https://datastore.googleapis.com" """Datastore API request URL base.""" + _USE_GRPC = _HAVE_GRPC and not os.getenv(DISABLE_GRPC, False) @@ -182,6 +193,14 @@ class Client(ClientWithProject): passed), falls back to the default inferred from the environment. + :type client_info: :class:`google.api_core.gapic_v1.client_info.ClientInfo` + or :class:`google.api_core.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 _http: :class:`~requests.Session` :param _http: (Optional) HTTP object to make requests. Can be any object that defines ``request()`` with the same interface as @@ -204,12 +223,19 @@ class Client(ClientWithProject): """The scopes required for authenticating as a Cloud Datastore consumer.""" def __init__( - self, project=None, namespace=None, credentials=None, _http=None, _use_grpc=None + self, + project=None, + namespace=None, + credentials=None, + client_info=_CLIENT_INFO, + _http=None, + _use_grpc=None, ): super(Client, self).__init__( project=project, credentials=credentials, _http=_http ) self.namespace = namespace + self._client_info = client_info self._batch_stack = _LocalStack() self._datastore_api_internal = None if _use_grpc is None: diff --git a/datastore/tests/unit/test__gapic.py b/datastore/tests/unit/test__gapic.py index ef359d4068e5..c404dc79c109 100644 --- a/datastore/tests/unit/test__gapic.py +++ b/datastore/tests/unit/test__gapic.py @@ -41,7 +41,8 @@ def test_live_api(self, make_chan, mock_klass): client = mock.Mock( _base_url=base_url, _credentials=mock.sentinel.credentials, - spec=["_base_url", "_credentials"], + _client_info=mock.sentinel.client_info, + spec=["_base_url", "_credentials", "_client_info"], ) ds_api = self._call_fut(client) self.assertIs(ds_api, mock.sentinel.ds_client) @@ -52,7 +53,7 @@ def test_live_api(self, make_chan, mock_klass): "datastore.googleapis.com:443", ) mock_klass.assert_called_once_with( - channel=mock.sentinel.channel, client_info=mock.ANY + channel=mock.sentinel.channel, client_info=mock.sentinel.client_info ) @mock.patch( @@ -70,12 +71,13 @@ def test_emulator(self, make_chan, mock_klass): client = mock.Mock( _base_url=base_url, _credentials=mock.sentinel.credentials, - spec=["_base_url", "_credentials"], + _client_info=mock.sentinel.client_info, + spec=["_base_url", "_credentials", "_client_info"], ) ds_api = self._call_fut(client) self.assertIs(ds_api, mock.sentinel.ds_client) make_chan.assert_called_once_with(host) mock_klass.assert_called_once_with( - channel=mock.sentinel.channel, client_info=mock.ANY + channel=mock.sentinel.channel, client_info=mock.sentinel.client_info ) diff --git a/datastore/tests/unit/test__http.py b/datastore/tests/unit/test__http.py index b402eafc9532..b332c946d40a 100644 --- a/datastore/tests/unit/test__http.py +++ b/datastore/tests/unit/test__http.py @@ -29,26 +29,27 @@ def _call_fut(*args, **kwargs): def test_success(self): from google.cloud import _http as connection_module - from google.cloud.datastore._http import _CLIENT_INFO project = "PROJECT" method = "METHOD" data = b"DATA" base_url = "http://api-url" + user_agent = "USER AGENT" + client_info = _make_client_info(user_agent) response_data = "CONTENT" http = _make_requests_session([_make_response(content=response_data)]) # Call actual function under test. - response = self._call_fut(http, project, method, data, base_url) + response = self._call_fut(http, project, method, data, base_url, client_info) self.assertEqual(response, response_data) # Check that the mocks were called as expected. expected_url = _build_expected_url(base_url, project, method) expected_headers = { "Content-Type": "application/x-protobuf", - "User-Agent": connection_module.DEFAULT_USER_AGENT, - connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO, + "User-Agent": user_agent, + connection_module.CLIENT_INFO_HEADER: user_agent, } http.request.assert_called_once_with( method="POST", url=expected_url, headers=expected_headers, data=data @@ -63,6 +64,8 @@ def test_failure(self): method = "METHOD" data = "DATA" uri = "http://api-url" + user_agent = "USER AGENT" + client_info = _make_client_info(user_agent) error = status_pb2.Status() error.message = "Entity value is indexed." @@ -73,7 +76,7 @@ def test_failure(self): ) with self.assertRaises(BadRequest) as exc: - self._call_fut(http, project, method, data, uri) + self._call_fut(http, project, method, data, uri, client_info) expected_message = "400 Entity value is indexed." self.assertEqual(str(exc.exception), expected_message) @@ -93,6 +96,7 @@ def test_it(self): project = "projectOK" method = "beginTransaction" base_url = "test.invalid" + client_info = _make_client_info() request_pb = datastore_pb2.BeginTransactionRequest(project_id=project) response_pb = datastore_pb2.BeginTransactionResponse(transaction=b"7830rmc") @@ -106,13 +110,19 @@ def test_it(self): project, method, base_url, + client_info, request_pb, datastore_pb2.BeginTransactionResponse, ) self.assertEqual(result, response_pb) mock_request.assert_called_once_with( - http, project, method, request_pb.SerializeToString(), base_url + http, + project, + method, + request_pb.SerializeToString(), + base_url, + client_info, ) @@ -149,8 +159,12 @@ def test_lookup_single_key_empty_response(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -182,8 +196,12 @@ def test_lookup_single_key_empty_response_w_eventual(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -214,8 +232,12 @@ def test_lookup_single_key_empty_response_w_transaction(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -249,8 +271,12 @@ def test_lookup_single_key_nonempty_response(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -284,8 +310,12 @@ def test_lookup_multiple_keys_empty_response(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -320,8 +350,12 @@ def test_lookup_multiple_keys_w_missing(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -355,8 +389,12 @@ def test_lookup_multiple_keys_w_deferred(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -399,8 +437,12 @@ def test_run_query_w_eventual_no_transaction(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -440,8 +482,12 @@ def test_run_query_wo_eventual_w_transaction(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -480,8 +526,12 @@ def test_run_query_wo_namespace_empty_result(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -522,8 +572,12 @@ def test_run_query_w_namespace_nonempty_result(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -550,8 +604,12 @@ def test_begin_transaction(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -586,8 +644,12 @@ def test_commit_wo_transaction(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -623,8 +685,12 @@ def test_commit_w_transaction(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -653,8 +719,12 @@ def test_rollback_ok(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -678,8 +748,12 @@ def test_allocate_ids_empty(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -711,8 +785,12 @@ def test_allocate_ids_non_empty(self): http = _make_requests_session( [_make_response(content=rsp_pb.SerializeToString())] ) + client_info = _make_client_info() client = mock.Mock( - _http=http, _base_url="test.invalid", spec=["_http", "_base_url"] + _http=http, + _base_url="test.invalid", + _client_info=client_info, + spec=["_http", "_base_url", "_client_info"], ) # Make request. @@ -760,14 +838,24 @@ def _make_key_pb(project, id_=1234): return Key(*path_args, project=project).to_protobuf() +_USER_AGENT = "TESTING USER AGENT" + + +def _make_client_info(user_agent=_USER_AGENT): + from google.api_core.client_info import ClientInfo + + client_info = mock.create_autospec(ClientInfo) + client_info.to_user_agent.return_value = user_agent + return client_info + + def _verify_protobuf_call(http, expected_url, pb): from google.cloud import _http as connection_module - from google.cloud.datastore._http import _CLIENT_INFO expected_headers = { "Content-Type": "application/x-protobuf", - "User-Agent": connection_module.DEFAULT_USER_AGENT, - connection_module.CLIENT_INFO_HEADER: _CLIENT_INFO, + "User-Agent": _USER_AGENT, + connection_module.CLIENT_INFO_HEADER: _USER_AGENT, } http.request.assert_called_once_with( diff --git a/datastore/tests/unit/test_client.py b/datastore/tests/unit/test_client.py index 10294db3cf62..05c6f4ddbfd5 100644 --- a/datastore/tests/unit/test_client.py +++ b/datastore/tests/unit/test_client.py @@ -127,6 +127,7 @@ def _make_one( project=PROJECT, namespace=None, credentials=None, + client_info=None, _http=None, _use_grpc=None, ): @@ -134,6 +135,7 @@ def _make_one( project=project, namespace=namespace, credentials=credentials, + client_info=client_info, _http=_http, _use_grpc=_use_grpc, ) @@ -148,6 +150,7 @@ def test_constructor_w_project_no_environ(self): self.assertRaises(EnvironmentError, self._make_one, None) def test_constructor_w_implicit_inputs(self): + from google.cloud.datastore.client import _CLIENT_INFO from google.cloud.datastore.client import _DATASTORE_BASE_URL other = "other" @@ -167,6 +170,7 @@ def test_constructor_w_implicit_inputs(self): self.assertEqual(client.project, other) self.assertIsNone(client.namespace) self.assertIs(client._credentials, creds) + self.assertIs(client._client_info, _CLIENT_INFO) self.assertIsNone(client._http_internal) self.assertEqual(client.base_url, _DATASTORE_BASE_URL) @@ -182,13 +186,19 @@ def test_constructor_w_explicit_inputs(self): other = "other" namespace = "namespace" creds = _make_credentials() + client_info = mock.Mock() http = object() client = self._make_one( - project=other, namespace=namespace, credentials=creds, _http=http + project=other, + namespace=namespace, + credentials=creds, + client_info=client_info, + _http=http, ) self.assertEqual(client.project, other) self.assertEqual(client.namespace, namespace) self.assertIs(client._credentials, creds) + self.assertIs(client._client_info, client_info) self.assertIs(client._http_internal, http) self.assertIsNone(client.current_batch) self.assertEqual(list(client._batch_stack), []) @@ -232,10 +242,29 @@ def test_constructor_gcd_host(self): client = self._make_one(project=project, credentials=creds, _http=http) self.assertEqual(client.base_url, "http://" + host) + def test_base_url_property(self): + alternate_url = "https://alias.example.com/" + project = "PROJECT" + creds = _make_credentials() + http = object() + + client = self._make_one(project=project, credentials=creds, _http=http) + client.base_url = alternate_url + self.assertEqual(client.base_url, alternate_url) + + def test__datastore_api_property_already_set(self): + client = self._make_one( + project="prahj-ekt", credentials=_make_credentials(), _use_grpc=True + ) + already = client._datastore_api_internal = object() + self.assertIs(client._datastore_api, already) + def test__datastore_api_property_gapic(self): + client_info = mock.Mock() client = self._make_one( project="prahj-ekt", credentials=_make_credentials(), + client_info=client_info, _http=object(), _use_grpc=True, ) @@ -247,41 +276,32 @@ def test__datastore_api_property_gapic(self): ) with patch as make_api: ds_api = client._datastore_api - self.assertIs(ds_api, mock.sentinel.ds_api) - make_api.assert_called_once_with(client) - self.assertIs(client._datastore_api_internal, mock.sentinel.ds_api) - # Make sure the cached value is used. - self.assertEqual(make_api.call_count, 1) - self.assertIs(client._datastore_api, mock.sentinel.ds_api) - self.assertEqual(make_api.call_count, 1) - - def test_base_url_property(self): - alternate_url = "https://alias.example.com/" - project = "PROJECT" - creds = _make_credentials() - http = object() - client = self._make_one(project=project, credentials=creds, _http=http) - client.base_url = alternate_url - self.assertEqual(client.base_url, alternate_url) + self.assertIs(ds_api, mock.sentinel.ds_api) + self.assertIs(client._datastore_api_internal, mock.sentinel.ds_api) + make_api.assert_called_once_with(client) def test__datastore_api_property_http(self): - from google.cloud.datastore._http import HTTPDatastoreAPI - + client_info = mock.Mock() client = self._make_one( project="prahj-ekt", credentials=_make_credentials(), + client_info=client_info, _http=object(), _use_grpc=False, ) self.assertIsNone(client._datastore_api_internal) - ds_api = client._datastore_api - self.assertIsInstance(ds_api, HTTPDatastoreAPI) - self.assertIs(ds_api.client, client) - # Make sure the cached value is used. - self.assertIs(client._datastore_api_internal, ds_api) - self.assertIs(client._datastore_api, ds_api) + patch = mock.patch( + "google.cloud.datastore.client.HTTPDatastoreAPI", + return_value=mock.sentinel.ds_api, + ) + with patch as make_api: + ds_api = client._datastore_api + + self.assertIs(ds_api, mock.sentinel.ds_api) + self.assertIs(client._datastore_api_internal, mock.sentinel.ds_api) + make_api.assert_called_once_with(client) def test__push_batch_and__pop_batch(self): creds = _make_credentials()