Skip to content

Commit 355fa75

Browse files
committed
De-coupling old client behavior from convenience methods.
This is so we can use them in Cloud Bigtable Client without the baggage of a connection class.
1 parent 556e365 commit 355fa75

File tree

5 files changed

+114
-80
lines changed

5 files changed

+114
-80
lines changed

docs/gcloud-api.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ Base Client
88
:members:
99
:undoc-members:
1010
:show-inheritance:
11+
:inherited-members:
1112

1213
Credentials Helpers
1314
~~~~~~~~~~~~~~~~~~~

gcloud/bigtable/client.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@
2727
"""
2828

2929

30-
from gcloud.client import JSONClient
30+
from gcloud.client import _ClientFactoryMixin
31+
from gcloud.client import _ClientProjectMixin
32+
from gcloud.credentials import get_credentials
3133

3234

3335
TABLE_ADMIN_HOST = 'bigtabletableadmin.googleapis.com'
@@ -54,7 +56,7 @@
5456
"""Scope for reading table data."""
5557

5658

57-
class Client(JSONClient):
59+
class Client(_ClientFactoryMixin, _ClientProjectMixin):
5860
"""Client for interacting with Google Cloud Bigtable API.
5961
6062
.. note::
@@ -88,18 +90,16 @@ class Client(JSONClient):
8890
and ``admin`` are :data:`True`
8991
"""
9092

91-
# Prevent creation of Client.connection.
92-
_connection_class = None
93-
9493
def __init__(self, project=None, credentials=None,
9594
read_only=False, admin=False):
95+
_ClientProjectMixin.__init__(self, project=project)
96+
if credentials is None:
97+
credentials = get_credentials()
98+
9699
if read_only and admin:
97100
raise ValueError('A read-only client cannot also perform'
98101
'administrative actions.')
99102

100-
super(Client, self).__init__(project=project, credentials=credentials)
101-
del self.http
102-
103103
scopes = []
104104
if read_only:
105105
scopes.append(READ_ONLY_SCOPE)
@@ -109,4 +109,4 @@ def __init__(self, project=None, credentials=None,
109109
if admin:
110110
scopes.append(ADMIN_SCOPE)
111111

112-
self.credentials = self.credentials.create_scoped(scopes)
112+
self.credentials = credentials.create_scoped(scopes)

gcloud/bigtable/test_client.py

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -25,42 +25,58 @@ def _getTargetClass(self):
2525
def _makeOne(self, *args, **kwargs):
2626
return self._getTargetClass()(*args, **kwargs)
2727

28-
def _constructor_test_helper(self, expected_scopes,
29-
read_only=False, admin=False):
28+
def _constructor_test_helper(self, expected_scopes, creds,
29+
read_only=False, admin=False,
30+
expected_creds=None):
3031
PROJECT = 'PROJECT'
31-
creds = _Credentials()
3232
client = self._makeOne(project=PROJECT, credentials=creds,
3333
read_only=read_only, admin=admin)
3434

35-
# When Client._connection_class is None, we have client.credentials
36-
# instead of client.connection.
37-
self.assertEqual(client.connection, None)
38-
self.assertTrue(client.credentials is creds)
39-
# Make sure scopes were passed as expected.
35+
expected_creds = expected_creds or creds
36+
self.assertTrue(client.credentials is expected_creds)
4037
self.assertEqual(client.project, PROJECT)
41-
self.assertEqual(creds._scopes, expected_scopes)
38+
self.assertEqual(client.credentials._scopes, expected_scopes)
4239

4340
def test_constructor_default_scopes(self):
4441
from gcloud.bigtable import client as MUT
4542

4643
expected_scopes = [MUT.DATA_SCOPE]
47-
self._constructor_test_helper(expected_scopes)
44+
creds = _Credentials()
45+
self._constructor_test_helper(expected_scopes, creds)
4846

4947
def test_constructor_with_admin(self):
5048
from gcloud.bigtable import client as MUT
5149

5250
expected_scopes = [MUT.DATA_SCOPE, MUT.ADMIN_SCOPE]
53-
self._constructor_test_helper(expected_scopes, admin=True)
51+
creds = _Credentials()
52+
self._constructor_test_helper(expected_scopes, creds, admin=True)
5453

5554
def test_constructor_with_read_only(self):
5655
from gcloud.bigtable import client as MUT
5756

5857
expected_scopes = [MUT.READ_ONLY_SCOPE]
59-
self._constructor_test_helper(expected_scopes, read_only=True)
58+
creds = _Credentials()
59+
self._constructor_test_helper(expected_scopes, creds, read_only=True)
6060

6161
def test_constructor_both_admin_and_read_only(self):
62+
creds = _Credentials()
6263
with self.assertRaises(ValueError):
63-
self._makeOne([], admin=True, read_only=True)
64+
self._constructor_test_helper([], creds, admin=True,
65+
read_only=True)
66+
67+
def test_constructor_implict_credentials(self):
68+
from gcloud._testing import _Monkey
69+
from gcloud.bigtable import client as MUT
70+
71+
creds = _Credentials()
72+
expected_scopes = [MUT.DATA_SCOPE]
73+
74+
def mock_get_credentials():
75+
return creds
76+
77+
with _Monkey(MUT, get_credentials=mock_get_credentials):
78+
self._constructor_test_helper(expected_scopes, None,
79+
expected_creds=creds)
6480

6581

6682
class _Credentials(object):

gcloud/client.py

Lines changed: 62 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -23,40 +23,16 @@
2323
from gcloud.credentials import get_for_service_account_p12
2424

2525

26-
class Client(object):
27-
"""Client to bundle configuration needed for API requests.
26+
class _ClientFactoryMixin(object):
27+
"""Mixin to allow factories that create credentials.
2828
29-
Assumes that the associated ``_connection_class`` only accepts
30-
``http`` and ``credentials`` in its constructor.
31-
32-
If ``_connection_class`` is :data:`None`, stores the ``credentials``
33-
on the client instead of the newly created ``connection``.
29+
.. note::
3430
35-
:type credentials: :class:`oauth2client.client.OAuth2Credentials` or
36-
:class:`NoneType`
37-
:param credentials: The OAuth2 Credentials to use for the connection
38-
owned by this client. If not passed (and if no ``http``
39-
object is passed), falls back to the default inferred
40-
from the environment.
41-
42-
:type http: :class:`httplib2.Http` or class that defines ``request()``.
43-
:param http: An optional HTTP object to make requests. If not passed, an
44-
``http`` object is created that is bound to the
45-
``credentials`` for the current object.
31+
This class is virtual.
4632
"""
4733

48-
_connection_class = Connection
49-
50-
def __init__(self, credentials=None, http=None):
51-
if credentials is None and http is None:
52-
credentials = get_credentials()
53-
self.connection = None
54-
if self._connection_class is not None:
55-
self.connection = self._connection_class(
56-
credentials=credentials, http=http)
57-
else:
58-
self.credentials = credentials
59-
self.http = http
34+
def __init__(self, *args, **kwargs):
35+
raise NotImplementedError('_ClientFactoryMixin is a virtual class')
6036

6137
@classmethod
6238
def from_service_account_json(cls, json_credentials_path, *args, **kwargs):
@@ -123,16 +99,14 @@ def from_service_account_p12(cls, client_email, private_key_path,
12399
return cls(*args, **kwargs)
124100

125101

126-
class JSONClient(Client):
127-
"""Client to for Google JSON-based API.
102+
class Client(_ClientFactoryMixin):
103+
"""Client to bundle configuration needed for API requests.
128104
129-
Assumes such APIs use the `project` and the client needs to store this
130-
value.
105+
Assumes that the associated ``_connection_class`` only accepts
106+
``http`` and ``credentials`` in its constructor.
131107
132-
:type project: string
133-
:param project: the project which the client acts on behalf of. If not
134-
passed falls back to the default inferred from the
135-
environment.
108+
If ``_connection_class`` is :data:`None`, stores the ``credentials``
109+
on the client instead of the newly created ``connection``.
136110
137111
:type credentials: :class:`oauth2client.client.OAuth2Credentials` or
138112
:class:`NoneType`
@@ -145,12 +119,30 @@ class JSONClient(Client):
145119
:param http: An optional HTTP object to make requests. If not passed, an
146120
``http`` object is created that is bound to the
147121
``credentials`` for the current object.
122+
"""
123+
124+
_connection_class = Connection
125+
126+
def __init__(self, credentials=None, http=None):
127+
if credentials is None and http is None:
128+
credentials = get_credentials()
129+
self.connection = self._connection_class(
130+
credentials=credentials, http=http)
131+
132+
133+
class _ClientProjectMixin(object):
134+
"""Mixin to allow setting the project on the client.
135+
136+
:type project: string
137+
:param project: the project which the client acts on behalf of. If not
138+
passed falls back to the default inferred from the
139+
environment.
148140
149141
:raises: :class:`ValueError` if the project is neither passed in nor
150142
set in the environment.
151143
"""
152144

153-
def __init__(self, project=None, credentials=None, http=None):
145+
def __init__(self, project=None):
154146
if project is None:
155147
project = _get_production_project()
156148
if project is None:
@@ -160,4 +152,34 @@ def __init__(self, project=None, credentials=None, http=None):
160152
raise ValueError('Project must be a string.')
161153
self.project = project
162154

163-
super(JSONClient, self).__init__(credentials=credentials, http=http)
155+
156+
class JSONClient(Client, _ClientProjectMixin):
157+
"""Client to for Google JSON-based API.
158+
159+
Assumes such APIs use the ``project`` and the client needs to store this
160+
value.
161+
162+
:type project: string
163+
:param project: the project which the client acts on behalf of. If not
164+
passed falls back to the default inferred from the
165+
environment.
166+
167+
:type credentials: :class:`oauth2client.client.OAuth2Credentials` or
168+
:class:`NoneType`
169+
:param credentials: The OAuth2 Credentials to use for the connection
170+
owned by this client. If not passed (and if no ``http``
171+
object is passed), falls back to the default inferred
172+
from the environment.
173+
174+
:type http: :class:`httplib2.Http` or class that defines ``request()``.
175+
:param http: An optional HTTP object to make requests. If not passed, an
176+
``http`` object is created that is bound to the
177+
``credentials`` for the current object.
178+
179+
:raises: :class:`ValueError` if the project is neither passed in nor
180+
set in the environment.
181+
"""
182+
183+
def __init__(self, project=None, credentials=None, http=None):
184+
_ClientProjectMixin.__init__(self, project=project)
185+
Client.__init__(self, credentials=credentials, http=http)

gcloud/test_client.py

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,19 @@
1515
import unittest2
1616

1717

18+
class Test_ClientFactoryMixin(unittest2.TestCase):
19+
20+
def _getTargetClass(self):
21+
from gcloud.client import _ClientFactoryMixin
22+
return _ClientFactoryMixin
23+
24+
def _makeOne(self, *args, **kw):
25+
return self._getTargetClass()(*args, **kw)
26+
27+
def test_virtual(self):
28+
self.assertRaises(NotImplementedError, self._makeOne)
29+
30+
1831
class TestClient(unittest2.TestCase):
1932

2033
def setUp(self):
@@ -50,8 +63,6 @@ def mock_get_credentials():
5063
self.assertTrue(isinstance(client_obj.connection, _MockConnection))
5164
self.assertTrue(client_obj.connection.credentials is CREDENTIALS)
5265
self.assertEqual(FUNC_CALLS, ['get_credentials'])
53-
self.assertFalse(hasattr(client_obj, 'credentials'))
54-
self.assertFalse(hasattr(client_obj, 'http'))
5566

5667
def test_ctor_explicit(self):
5768
CREDENTIALS = object()
@@ -61,22 +72,6 @@ def test_ctor_explicit(self):
6172
self.assertTrue(isinstance(client_obj.connection, _MockConnection))
6273
self.assertTrue(client_obj.connection.credentials is CREDENTIALS)
6374
self.assertTrue(client_obj.connection.http is HTTP)
64-
self.assertFalse(hasattr(client_obj, 'credentials'))
65-
self.assertFalse(hasattr(client_obj, 'http'))
66-
67-
def test_ctor_without_connection_class(self):
68-
klass = self._getTargetClass()
69-
70-
class _ClientLite(klass):
71-
_connection_class = None
72-
73-
CREDENTIALS = object()
74-
HTTP = object()
75-
client_obj = _ClientLite(credentials=CREDENTIALS, http=HTTP)
76-
77-
self.assertEqual(client_obj.connection, None)
78-
self.assertEqual(client_obj.credentials, CREDENTIALS)
79-
self.assertEqual(client_obj.http, HTTP)
8075

8176
def test_from_service_account_json(self):
8277
from gcloud._testing import _Monkey

0 commit comments

Comments
 (0)