Skip to content

Simplifying usage of Client classes in Spanner and Bigtable #3671

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.

Sign up for GitHub

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

Closed
dhermes wants to merge 2 commits into from
Closed

Simplifying usage of Client classes in Spanner and Bigtable #3671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
70bdd8e
Simplifying Client constructor's for Bigtable and Spanner.
dhermes Jul 25, 2017
ca0f1e1
Collapsing base Client classes.
dhermes Jul 25, 2017
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
48 changes: 24 additions & 24 deletions bigtable/google/cloud/bigtable/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,14 @@

import os

import google.auth
import google.auth.credentials
from google.gax.utils import metrics
from google.longrunning import operations_grpc

from google.cloud._helpers import make_insecure_stub
from google.cloud._helpers import make_secure_stub
from google.cloud._http import DEFAULT_USER_AGENT
from google.cloud.client import _ClientFactoryMixin
from google.cloud.client import _ClientProjectMixin
from google.cloud.client import ClientWithProject
from google.cloud.environment_vars import BIGTABLE_EMULATOR

from google.cloud.bigtable import __version__
Expand Down Expand Up @@ -166,13 +164,13 @@ def _make_table_stub(client):
client.emulator_host)


class Client(_ClientFactoryMixin, _ClientProjectMixin):
class Client(ClientWithProject):
"""Client for interacting with Google Cloud Bigtable API.

.. note::

Since the Cloud Bigtable API requires the gRPC transport, no
``http`` argument is accepted by this class.
``_http`` argument is accepted by this class.

:type project: :class:`str` or :func:`unicode <unicode>`
:param project: (Optional) The ID of the project which owns the
Expand Down Expand Up @@ -209,31 +207,17 @@ class Client(_ClientFactoryMixin, _ClientProjectMixin):

def __init__(self, project=None, credentials=None,
read_only=False, admin=False, user_agent=DEFAULT_USER_AGENT):
_ClientProjectMixin.__init__(self, project=project)
if credentials is None:
credentials, _ = google.auth.default()

if read_only and admin:
raise ValueError('A read-only client cannot also perform'
'administrative actions.')

scopes = []
if read_only:
scopes.append(READ_ONLY_SCOPE)
else:
scopes.append(DATA_SCOPE)

# NOTE: This API has no use for the _http argument, but sending it
# will have no impact since the _http() @property only lazily
# creates a working HTTP object.
super(Client, self).__init__(
project=project, credentials=credentials, _http=None)
self._read_only = bool(read_only)

if admin:
scopes.append(ADMIN_SCOPE)

self._admin = bool(admin)

credentials = google.auth.credentials.with_scopes_if_required(
credentials, scopes)

self._credentials = credentials
self.user_agent = user_agent
self.emulator_host = os.getenv(BIGTABLE_EMULATOR)

Expand All @@ -244,6 +228,22 @@ def __init__(self, project=None, credentials=None,
self._operations_stub_internal = _make_operations_stub(self)
self._table_stub_internal = _make_table_stub(self)

self._set_scopes()

def _set_scopes(self):
"""Set the scopes on the current credentials."""
scopes = []
if self._read_only:
scopes.append(READ_ONLY_SCOPE)
else:
scopes.append(DATA_SCOPE)

if self._admin:
scopes.append(ADMIN_SCOPE)

self._credentials = google.auth.credentials.with_scopes_if_required(
self._credentials, scopes)

def copy(self):
"""Make a copy of this client.

Expand Down
128 changes: 50 additions & 78 deletions core/google/cloud/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

import google.auth
import google.auth.credentials
from google.cloud._helpers import _determine_default_project
from google.cloud import _helpers
from google.oauth2 import service_account


Expand All @@ -34,54 +34,7 @@
)


class _ClientFactoryMixin(object):
"""Mixin to allow factories that create credentials.

.. note::

This class is virtual.
"""

_SET_PROJECT = False

@classmethod
def from_service_account_json(cls, json_credentials_path, *args, **kwargs):
"""Factory to retrieve JSON credentials while creating client.

:type json_credentials_path: str
:param json_credentials_path: The path to a private key file (this file
was given to you when you created the
service account). This file must contain
a JSON object with a private key and
other credentials information (downloaded
from the Google APIs console).

:type args: tuple
:param args: Remaining positional arguments to pass to constructor.

:type kwargs: dict
:param kwargs: Remaining keyword arguments to pass to constructor.

:rtype: :class:`_ClientFactoryMixin`
:returns: The client created with the retrieved JSON credentials.
:raises TypeError: if there is a conflict with the kwargs
and the credentials created by the factory.
"""
if 'credentials' in kwargs:
raise TypeError('credentials must not be in keyword arguments')
with io.open(json_credentials_path, 'r', encoding='utf-8') as json_fi:
credentials_info = json.load(json_fi)
credentials = service_account.Credentials.from_service_account_info(
credentials_info)
if cls._SET_PROJECT:
if 'project' not in kwargs:
kwargs['project'] = credentials_info.get('project_id')

kwargs['credentials'] = credentials
return cls(*args, **kwargs)


class Client(_ClientFactoryMixin):
class Client(object):
"""Client to bundle configuration needed for API requests.

Stores ``credentials`` and an HTTP object so that subclasses
Expand Down Expand Up @@ -123,6 +76,8 @@ class Client(_ClientFactoryMixin):
change in the future.
"""

_SET_PROJECT = False

SCOPE = None
"""The scopes required for authenticating with a service.

Expand Down Expand Up @@ -159,38 +114,44 @@ def _http(self):
self._credentials)
return self._http_internal

@classmethod
def from_service_account_json(cls, json_credentials_path, *args, **kwargs):
"""Factory to retrieve JSON credentials while creating client.

class _ClientProjectMixin(object):
"""Mixin to allow setting the project on the client.
:type json_credentials_path: str
:param json_credentials_path: The path to a private key file (this file
was given to you when you created the
service account). This file must contain
a JSON object with a private key and
other credentials information (downloaded
from the Google APIs console).

:type project: str
:param project: the project which the client acts on behalf of. If not
passed falls back to the default inferred from the
environment.
:type args: tuple
:param args: Remaining positional arguments to pass to constructor.

:raises: :class:`EnvironmentError` if the project is neither passed in nor
set in the environment. :class:`ValueError` if the project value
is invalid.
"""
:type kwargs: dict
:param kwargs: Remaining keyword arguments to pass to constructor.

def __init__(self, project=None):
project = self._determine_default(project)
if project is None:
raise EnvironmentError('Project was not passed and could not be '
'determined from the environment.')
if isinstance(project, six.binary_type):
project = project.decode('utf-8')
if not isinstance(project, six.string_types):
raise ValueError('Project must be a string.')
self.project = project
:rtype: :class:`Client`
:returns: The client created with the retrieved JSON credentials.
:raises TypeError: if there is a conflict with the kwargs
and the credentials created by the factory.
"""
if 'credentials' in kwargs:
raise TypeError('credentials must not be in keyword arguments')
with io.open(json_credentials_path, 'r', encoding='utf-8') as json_fi:
credentials_info = json.load(json_fi)
credentials = service_account.Credentials.from_service_account_info(
credentials_info)
if cls._SET_PROJECT:
if 'project' not in kwargs:
kwargs['project'] = credentials_info.get('project_id')

@staticmethod
def _determine_default(project):
"""Helper: use default project detection."""
return _determine_default_project(project)
kwargs['credentials'] = credentials
return cls(*args, **kwargs)


class ClientWithProject(Client, _ClientProjectMixin):
class ClientWithProject(Client):
"""Client that also stores a project.

:type project: str
Expand All @@ -213,12 +174,23 @@ class ClientWithProject(Client, _ClientProjectMixin):
This parameter should be considered private, and could
change in the future.

:raises: :class:`ValueError` if the project is neither passed in nor
set in the environment.
:raises EnvironmentError: If the project is neither passed in nor
set in the environment.
"""

_SET_PROJECT = True # Used by from_service_account_json()

def __init__(self, project=None, credentials=None, _http=None):
_ClientProjectMixin.__init__(self, project=project)
Client.__init__(self, credentials=credentials, _http=_http)
super(ClientWithProject, self).__init__(
credentials=credentials, _http=_http)

project = self._determine_default(project)
if project is None:
raise EnvironmentError('Project was not passed and could not be '
'determined from the environment.')
self.project = _helpers._bytes_to_unicode(project)

@staticmethod
def _determine_default(project):
"""Helper: use default project detection."""
return _helpers._determine_default_project(project)
13 changes: 0 additions & 13 deletions core/tests/unit/test_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,19 +25,6 @@ def _make_credentials():
return mock.Mock(spec=google.auth.credentials.Credentials)


class Test_ClientFactoryMixin(unittest.TestCase):

@staticmethod
def _get_target_class():
from google.cloud.client import _ClientFactoryMixin

return _ClientFactoryMixin

def test_virtual(self):
klass = self._get_target_class()
self.assertFalse('__init__' in klass.__dict__)


class TestClient(unittest.TestCase):

@staticmethod
Expand Down
28 changes: 11 additions & 17 deletions spanner/google/cloud/spanner/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,7 @@
# pylint: enable=line-too-long

from google.cloud._http import DEFAULT_USER_AGENT
from google.cloud.client import _ClientFactoryMixin
from google.cloud.client import _ClientProjectMixin
from google.cloud.client import ClientWithProject
from google.cloud.iterator import GAXIterator
from google.cloud.spanner import __version__
from google.cloud.spanner._helpers import _options_with_prefix
Expand Down Expand Up @@ -73,13 +72,13 @@ def from_pb(cls, config_pb):
return cls(config_pb.name, config_pb.display_name)


class Client(_ClientFactoryMixin, _ClientProjectMixin):
class Client(ClientWithProject):
"""Client for interacting with Cloud Spanner API.

.. note::

Since the Cloud Spanner API requires the gRPC transport, no
``http`` argument is accepted by this class.
``_http`` argument is accepted by this class.

:type project: :class:`str` or :func:`unicode <unicode>`
:param project: (Optional) The ID of the project which owns the
Expand All @@ -104,21 +103,16 @@ class Client(_ClientFactoryMixin, _ClientProjectMixin):
_database_admin_api = None
_SET_PROJECT = True # Used by from_service_account_json()

SCOPE = (SPANNER_ADMIN_SCOPE,)
"""The scopes required for Google Cloud Spanner."""

def __init__(self, project=None, credentials=None,
user_agent=DEFAULT_USER_AGENT):

_ClientProjectMixin.__init__(self, project=project)
if credentials is None:
credentials, _ = google.auth.default()

scopes = [
SPANNER_ADMIN_SCOPE,
]

credentials = google.auth.credentials.with_scopes_if_required(
credentials, scopes)

self._credentials = credentials
# NOTE: This API has no use for the _http argument, but sending it
# will have no impact since the _http() @property only lazily
# creates a working HTTP object.
super(Client, self).__init__(
project=project, credentials=credentials, _http=None)
self.user_agent = user_agent

@property
Expand Down