Skip to content

Commit

Permalink
Bigtable: deprecate 'channel' arg to 'Client' (#6279)
Browse files Browse the repository at this point in the history
We can't do anything with it, because it conflicts with the credentials
we (always) have.

Also, add explicit tests for 'Client' constructor, including assertions
for all attributes.
  • Loading branch information
tseaver committed Oct 23, 2018
1 parent 6baadd8 commit b34a859
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 57 deletions.
19 changes: 12 additions & 7 deletions bigtable/google/cloud/bigtable/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
* a :class:`~google.cloud.bigtable.table.Table` owns a
:class:`~google.cloud.bigtable.row.Row` (and all the cells in the row)
"""

import warnings

from google.api_core.gapic_v1 import client_info

Expand Down Expand Up @@ -87,10 +87,10 @@ class Client(ClientWithProject):
requires the :const:`ADMIN_SCOPE`. Defaults to :data:`False`.
:type channel: :instance: grpc.Channel
:param channel (grpc.Channel): (Optional) A ``Channel`` instance
through which to make calls. This argument is mutually
exclusive with ``credentials``; providing both will raise an
exception.
:param channel (grpc.Channel): (Optional) DEPRECATED:
A ``Channel`` instance through which to make calls.
This argument is mutually exclusive with ``credentials``;
providing both will raise an exception. No longer used.
:raises: :class:`ValueError <exceptions.ValueError>` if both ``read_only``
and ``admin`` are :data:`True`
Expand All @@ -109,6 +109,12 @@ def __init__(self, project=None, credentials=None,
# It **may** use those scopes in ``with_scopes_if_required``.
self._read_only = bool(read_only)
self._admin = bool(admin)

if channel is not None:
warnings.warn(
"'channel' is deprecated and no longer used.",
DeprecationWarning, stacklevel=2)

self._channel = channel
self.SCOPE = self._get_scopes()
super(Client, self).__init__(project=project, credentials=credentials)
Expand Down Expand Up @@ -145,8 +151,7 @@ def project_path(self):
:rtype: str
:returns: Return a fully-qualified project string.
"""
instance_client = self.instance_admin_client
return instance_client.project_path(self.project)
return self.instance_admin_client.project_path(self.project)

@property
def table_data_client(self):
Expand Down
135 changes: 85 additions & 50 deletions bigtable/tests/unit/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,49 @@ def _get_target_class():
def _make_one(self, *args, **kwargs):
return self._get_target_class()(*args, **kwargs)

def test_constructor_defaults(self):
from google.cloud.bigtable.client import DATA_SCOPE

credentials = _make_credentials()

with mock.patch('google.auth.default') as mocked:
mocked.return_value = credentials, self.PROJECT
client = self._make_one()

self.assertEqual(client.project, self.PROJECT)
self.assertIs(
client._credentials, credentials.with_scopes.return_value)
self.assertFalse(client._read_only)
self.assertFalse(client._admin)
self.assertIsNone(client._channel)
self.assertEqual(client.SCOPE, (DATA_SCOPE,))

def test_constructor_explicit(self):
import warnings
from google.cloud.bigtable.client import ADMIN_SCOPE
from google.cloud.bigtable.client import DATA_SCOPE

credentials = _make_credentials()

with warnings.catch_warnings(record=True) as warned:
client = self._make_one(
project=self.PROJECT,
credentials=credentials,
read_only=False,
admin=True,
channel=mock.sentinel.channel,
)

self.assertEqual(len(warned), 1)

self.assertEqual(client.project, self.PROJECT)
self.assertIs(
client._credentials, credentials.with_scopes.return_value)
self.assertFalse(client._read_only)
self.assertTrue(client._admin)
self.assertIs(client._channel, mock.sentinel.channel)
self.assertEqual(client.SCOPE, (DATA_SCOPE, ADMIN_SCOPE))

def test_constructor_both_admin_and_read_only(self):
credentials = _make_credentials()
with self.assertRaises(ValueError):
Expand Down Expand Up @@ -68,63 +111,14 @@ def test__get_scopes_read_only(self):
read_only=True)
self.assertEqual(client._get_scopes(), (READ_ONLY_SCOPE,))

def test_credentials_getter(self):
credentials = _make_credentials()
project = 'PROJECT'
client = self._make_one(
project=project, credentials=credentials)
self.assertIs(client._credentials,
credentials.with_scopes.return_value)

def test_project_name_property(self):
def test_project_path_property(self):
credentials = _make_credentials()
project = 'PROJECT'
client = self._make_one(project=project, credentials=credentials,
admin=True)
project_name = 'projects/' + project
self.assertEqual(client.project_path, project_name)

def test_instance_factory_defaults(self):
from google.cloud.bigtable.instance import Instance

PROJECT = 'PROJECT'
INSTANCE_ID = 'instance-id'
credentials = _make_credentials()
client = self._make_one(
project=PROJECT, credentials=credentials)

instance = client.instance(INSTANCE_ID)

self.assertIsInstance(instance, Instance)
self.assertEqual(instance.instance_id, INSTANCE_ID)
self.assertEqual(instance.display_name, INSTANCE_ID)
self.assertIsNone(instance.type_)
self.assertIsNone(instance.labels)
self.assertIs(instance._client, client)

def test_instance_factory_non_defaults(self):
from google.cloud.bigtable.instance import Instance
from google.cloud.bigtable import enums

PROJECT = 'PROJECT'
INSTANCE_ID = 'instance-id'
DISPLAY_NAME = 'display-name'
instance_type = enums.Instance.Type.DEVELOPMENT
labels = {'foo': 'bar'}
credentials = _make_credentials()
client = self._make_one(
project=PROJECT, credentials=credentials)

instance = client.instance(INSTANCE_ID, display_name=DISPLAY_NAME,
instance_type=instance_type, labels=labels)

self.assertIsInstance(instance, Instance)
self.assertEqual(instance.instance_id, INSTANCE_ID)
self.assertEqual(instance.display_name, DISPLAY_NAME)
self.assertEqual(instance.type_, instance_type)
self.assertEqual(instance.labels, labels)
self.assertIs(instance._client, client)

def test_table_data_client_not_initialized(self):
from google.cloud.bigtable_v2 import BigtableClient

Expand Down Expand Up @@ -194,6 +188,47 @@ def test_instance_admin_client_initialized(self):
already = client._instance_admin_client = object()
self.assertIs(client.instance_admin_client, already)

def test_instance_factory_defaults(self):
from google.cloud.bigtable.instance import Instance

PROJECT = 'PROJECT'
INSTANCE_ID = 'instance-id'
credentials = _make_credentials()
client = self._make_one(
project=PROJECT, credentials=credentials)

instance = client.instance(INSTANCE_ID)

self.assertIsInstance(instance, Instance)
self.assertEqual(instance.instance_id, INSTANCE_ID)
self.assertEqual(instance.display_name, INSTANCE_ID)
self.assertIsNone(instance.type_)
self.assertIsNone(instance.labels)
self.assertIs(instance._client, client)

def test_instance_factory_non_defaults(self):
from google.cloud.bigtable.instance import Instance
from google.cloud.bigtable import enums

PROJECT = 'PROJECT'
INSTANCE_ID = 'instance-id'
DISPLAY_NAME = 'display-name'
instance_type = enums.Instance.Type.DEVELOPMENT
labels = {'foo': 'bar'}
credentials = _make_credentials()
client = self._make_one(
project=PROJECT, credentials=credentials)

instance = client.instance(INSTANCE_ID, display_name=DISPLAY_NAME,
instance_type=instance_type, labels=labels)

self.assertIsInstance(instance, Instance)
self.assertEqual(instance.instance_id, INSTANCE_ID)
self.assertEqual(instance.display_name, DISPLAY_NAME)
self.assertEqual(instance.type_, instance_type)
self.assertEqual(instance.labels, labels)
self.assertIs(instance._client, client)

def test_list_instances(self):
from google.cloud.bigtable_admin_v2.proto import (
instance_pb2 as data_v2_pb2)
Expand Down

0 comments on commit b34a859

Please sign in to comment.