-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Bigtable: add 'client_info' support to client. #7876
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,10 +63,12 @@ | |
def _create_gapic_client(client_class): | ||
def inner(self): | ||
if self._emulator_host is None: | ||
return client_class(credentials=self._credentials, client_info=_CLIENT_INFO) | ||
return client_class( | ||
credentials=self._credentials, client_info=self._client_info | ||
) | ||
else: | ||
return client_class( | ||
channel=self._emulator_channel, client_info=_CLIENT_INFO | ||
channel=self._emulator_channel, client_info=self._client_info | ||
) | ||
|
||
return inner | ||
|
@@ -100,6 +102,13 @@ class Client(ClientWithProject): | |
interact with the Instance Admin or Table Admin APIs. This | ||
requires the :const:`ADMIN_SCOPE`. Defaults to :data:`False`. | ||
|
||
:type: client_info: :class:`google.api_core.client_info.ClientInfo` | ||
:param client_info: | ||
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 channel: :instance: grpc.Channel | ||
:param channel (grpc.Channel): (Optional) DEPRECATED: | ||
A ``Channel`` instance through which to make calls. | ||
|
@@ -115,7 +124,13 @@ class Client(ClientWithProject): | |
_instance_admin_client = None | ||
|
||
def __init__( | ||
self, project=None, credentials=None, read_only=False, admin=False, channel=None | ||
self, | ||
project=None, | ||
credentials=None, | ||
read_only=False, | ||
admin=False, | ||
client_info=_CLIENT_INFO, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since |
||
channel=None, | ||
): | ||
if read_only and admin: | ||
raise ValueError( | ||
|
@@ -126,6 +141,7 @@ def __init__( | |
# It **may** use those scopes in ``with_scopes_if_required``. | ||
self._read_only = bool(read_only) | ||
self._admin = bool(admin) | ||
self._client_info = client_info | ||
self._emulator_host = os.getenv(BIGTABLE_EMULATOR) | ||
self._emulator_channel = None | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,34 +27,32 @@ def _invoke_client_factory(self, client_class): | |
return _create_gapic_client(client_class) | ||
|
||
def test_without_emulator(self): | ||
from google.cloud.bigtable.client import _CLIENT_INFO | ||
|
||
client_class = mock.Mock() | ||
credentials = _make_credentials() | ||
client = _Client(credentials) | ||
client_info = client._client_info = mock.Mock() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe use an autospec here? |
||
|
||
result = self._invoke_client_factory(client_class)(client) | ||
|
||
self.assertIs(result, client_class.return_value) | ||
client_class.assert_called_once_with( | ||
credentials=client._credentials, client_info=_CLIENT_INFO | ||
credentials=client._credentials, client_info=client_info | ||
) | ||
|
||
def test_with_emulator(self): | ||
from google.cloud.bigtable.client import _CLIENT_INFO | ||
|
||
client_class = mock.Mock() | ||
emulator_host = emulator_channel = object() | ||
credentials = _make_credentials() | ||
client = _Client( | ||
credentials, emulator_host=emulator_host, emulator_channel=emulator_channel | ||
) | ||
client_info = client._client_info = mock.Mock() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto. We can use |
||
|
||
result = self._invoke_client_factory(client_class)(client) | ||
|
||
self.assertIs(result, client_class.return_value) | ||
client_class.assert_called_once_with( | ||
channel=client._emulator_channel, client_info=_CLIENT_INFO | ||
channel=client._emulator_channel, client_info=client_info | ||
) | ||
|
||
|
||
|
@@ -82,6 +80,7 @@ def _make_one(self, *args, **kwargs): | |
return self._get_target_class()(*args, **kwargs) | ||
|
||
def test_constructor_defaults(self): | ||
from google.cloud.bigtable.client import _CLIENT_INFO | ||
from google.cloud.bigtable.client import DATA_SCOPE | ||
|
||
credentials = _make_credentials() | ||
|
@@ -94,6 +93,7 @@ def test_constructor_defaults(self): | |
self.assertIs(client._credentials, credentials.with_scopes.return_value) | ||
self.assertFalse(client._read_only) | ||
self.assertFalse(client._admin) | ||
self.assertIs(client._client_info, _CLIENT_INFO) | ||
self.assertIsNone(client._channel) | ||
self.assertIsNone(client._emulator_host) | ||
self.assertIsNone(client._emulator_channel) | ||
|
@@ -105,13 +105,15 @@ def test_constructor_explicit(self): | |
from google.cloud.bigtable.client import DATA_SCOPE | ||
|
||
credentials = _make_credentials() | ||
client_info = mock.Mock() | ||
|
||
with warnings.catch_warnings(record=True) as warned: | ||
client = self._make_one( | ||
project=self.PROJECT, | ||
credentials=credentials, | ||
read_only=False, | ||
admin=True, | ||
client_info=client_info, | ||
channel=mock.sentinel.channel, | ||
) | ||
|
||
|
@@ -121,6 +123,7 @@ def test_constructor_explicit(self): | |
self.assertIs(client._credentials, credentials.with_scopes.return_value) | ||
self.assertFalse(client._read_only) | ||
self.assertTrue(client._admin) | ||
self.assertIs(client._client_info, client_info) | ||
self.assertIs(client._channel, mock.sentinel.channel) | ||
self.assertEqual(client.SCOPE, (DATA_SCOPE, ADMIN_SCOPE)) | ||
|
||
|
@@ -182,13 +185,29 @@ def test_project_path_property(self): | |
self.assertEqual(client.project_path, project_name) | ||
|
||
def test_table_data_client_not_initialized(self): | ||
from google.cloud.bigtable.client import _CLIENT_INFO | ||
from google.cloud.bigtable_v2 import BigtableClient | ||
|
||
credentials = _make_credentials() | ||
client = self._make_one(project=self.PROJECT, credentials=credentials) | ||
|
||
table_data_client = client.table_data_client | ||
self.assertIsInstance(table_data_client, BigtableClient) | ||
self.assertIs(table_data_client._client_info, _CLIENT_INFO) | ||
self.assertIs(client._table_data_client, table_data_client) | ||
|
||
def test_table_data_client_not_initialized_w_client_info(self): | ||
from google.cloud.bigtable_v2 import BigtableClient | ||
|
||
credentials = _make_credentials() | ||
client_info = mock.Mock() | ||
client = self._make_one( | ||
project=self.PROJECT, credentials=credentials, client_info=client_info | ||
) | ||
|
||
table_data_client = client.table_data_client | ||
self.assertIsInstance(table_data_client, BigtableClient) | ||
self.assertIs(table_data_client._client_info, client_info) | ||
self.assertIs(client._table_data_client, table_data_client) | ||
|
||
def test_table_data_client_initialized(self): | ||
|
@@ -208,6 +227,7 @@ def test_table_admin_client_not_initialized_no_admin_flag(self): | |
client.table_admin_client() | ||
|
||
def test_table_admin_client_not_initialized_w_admin_flag(self): | ||
from google.cloud.bigtable.client import _CLIENT_INFO | ||
from google.cloud.bigtable_admin_v2 import BigtableTableAdminClient | ||
|
||
credentials = _make_credentials() | ||
|
@@ -217,6 +237,25 @@ def test_table_admin_client_not_initialized_w_admin_flag(self): | |
|
||
table_admin_client = client.table_admin_client | ||
self.assertIsInstance(table_admin_client, BigtableTableAdminClient) | ||
self.assertIs(table_admin_client._client_info, _CLIENT_INFO) | ||
self.assertIs(client._table_admin_client, table_admin_client) | ||
|
||
def test_table_admin_client_not_initialized_w_client_info(self): | ||
from google.cloud.bigtable_admin_v2 import BigtableTableAdminClient | ||
|
||
credentials = _make_credentials() | ||
client_info = mock.Mock() | ||
client = self._make_one( | ||
project=self.PROJECT, | ||
credentials=credentials, | ||
admin=True, | ||
client_info=client_info, | ||
) | ||
|
||
table_admin_client = client.table_admin_client | ||
self.assertIsInstance(table_admin_client, BigtableTableAdminClient) | ||
self.assertIs(table_admin_client._client_info, client_info) | ||
self.assertIs(client._table_admin_client, table_admin_client) | ||
|
||
def test_table_admin_client_initialized(self): | ||
credentials = _make_credentials() | ||
|
@@ -235,6 +274,7 @@ def test_instance_admin_client_not_initialized_no_admin_flag(self): | |
client.instance_admin_client() | ||
|
||
def test_instance_admin_client_not_initialized_w_admin_flag(self): | ||
from google.cloud.bigtable.client import _CLIENT_INFO | ||
from google.cloud.bigtable_admin_v2 import BigtableInstanceAdminClient | ||
|
||
credentials = _make_credentials() | ||
|
@@ -244,6 +284,25 @@ def test_instance_admin_client_not_initialized_w_admin_flag(self): | |
|
||
instance_admin_client = client.instance_admin_client | ||
self.assertIsInstance(instance_admin_client, BigtableInstanceAdminClient) | ||
self.assertIs(instance_admin_client._client_info, _CLIENT_INFO) | ||
self.assertIs(client._instance_admin_client, instance_admin_client) | ||
|
||
def test_instance_admin_client_not_initialized_w_admin_and_client_info(self): | ||
from google.cloud.bigtable_admin_v2 import BigtableInstanceAdminClient | ||
|
||
credentials = _make_credentials() | ||
client_info = mock.Mock() | ||
client = self._make_one( | ||
project=self.PROJECT, | ||
credentials=credentials, | ||
admin=True, | ||
client_info=client_info, | ||
) | ||
|
||
instance_admin_client = client.instance_admin_client | ||
self.assertIsInstance(instance_admin_client, BigtableInstanceAdminClient) | ||
self.assertIs(instance_admin_client._client_info, client_info) | ||
self.assertIs(client._instance_admin_client, instance_admin_client) | ||
|
||
def test_instance_admin_client_initialized(self): | ||
credentials = _make_credentials() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be the
google.api_core.gapic_v1.client_info.ClientInfo
version, instead?What happens when a
google.api_core.client_info.ClientInfo
is used? I imagine it might fail to populate thex-goog-api-client
header?