Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Merged
merged 1 commit 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
22 changes: 19 additions & 3 deletions 22 bigtable/google/cloud/bigtable/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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`
Copy link
Contributor

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 the x-goog-api-client header?

: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.
Expand All @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since _CLIENT_INFO is mutable, should we be defaulting to None and making a copy of _CLIENT_INFO if not set? I guess we don't expect anyone to be mutating it, since _CLIENT_INFO and self._client_info are both private?

channel=None,
):
if read_only and admin:
raise ValueError(
Expand All @@ -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

Expand Down
71 changes: 65 additions & 6 deletions 71 bigtable/tests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Contributor

Choose a reason for hiding this comment

The 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()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto. We can use mock.create_autospec with instance=True to make doubly sure we don't use any properties on ClientInfo that don't exist.


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
)


Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand All @@ -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,
)

Expand All @@ -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))

Expand Down Expand Up @@ -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):
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.