From 77f19bdc47442abbe3cb004affb7f057eb237d87 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Thu, 19 Feb 2015 13:02:07 -0800 Subject: [PATCH 1/4] Remove datastore.helpers import in connection. Done via: - Moving `_add_keys_to_request` definition to connection module - Removing only other use (in `batch`, was not a list of keys) - Copying `_prepare_key_for_request` from helpers (#528 would be nice, since we could just remove `_prepare_key_for_request` all together) --- gcloud/datastore/batch.py | 4 ++-- gcloud/datastore/connection.py | 41 +++++++++++++++++++++++++++++++--- gcloud/datastore/helpers.py | 14 ------------ 3 files changed, 40 insertions(+), 19 deletions(-) diff --git a/gcloud/datastore/batch.py b/gcloud/datastore/batch.py index bf2f04f63e24..2accbc02cddd 100644 --- a/gcloud/datastore/batch.py +++ b/gcloud/datastore/batch.py @@ -189,8 +189,8 @@ def delete(self, key): if not _dataset_ids_equal(self._dataset_id, key.dataset_id): raise ValueError("Key must be from same dataset as batch") - key_pb = key.to_protobuf() - helpers._add_keys_to_request(self.mutation.delete, [key_pb]) + key_pb = helpers._prepare_key_for_request(key.to_protobuf()) + self.mutation.delete.add().CopyFrom(key_pb) def begin(self): """No-op diff --git a/gcloud/datastore/connection.py b/gcloud/datastore/connection.py index 4e5d569c02f5..57724fec0c54 100644 --- a/gcloud/datastore/connection.py +++ b/gcloud/datastore/connection.py @@ -19,7 +19,6 @@ from gcloud import connection from gcloud.exceptions import make_exception from gcloud.datastore import _datastore_v1_pb2 as datastore_pb -from gcloud.datastore import helpers _GCD_HOST_ENV_VAR_NAME = 'DATASTORE_HOST' @@ -183,7 +182,7 @@ def lookup(self, dataset_id, key_pbs, """ lookup_request = datastore_pb.LookupRequest() _set_read_options(lookup_request, eventual, transaction_id) - helpers._add_keys_to_request(lookup_request.key, key_pbs) + _add_keys_to_request(lookup_request.key, key_pbs) lookup_response = self._rpc(dataset_id, 'lookup', lookup_request, datastore_pb.LookupResponse) @@ -363,7 +362,7 @@ def allocate_ids(self, dataset_id, key_pbs): :returns: An equal number of keys, with IDs filled in by the backend. """ request = datastore_pb.AllocateIdsRequest() - helpers._add_keys_to_request(request.key, key_pbs) + _add_keys_to_request(request.key, key_pbs) # Nothing to do with this response, so just execute the method. response = self._rpc(dataset_id, 'allocateIds', request, datastore_pb.AllocateIdsResponse) @@ -386,3 +385,39 @@ def _set_read_options(request, eventual, transaction_id): opts.read_consistency = datastore_pb.ReadOptions.EVENTUAL elif transaction_id: opts.transaction = transaction_id + + +def _prepare_key_for_request(key_pb): # pragma: NO COVER copied from helpers + """Add protobuf keys to a request object. + + .. note:: + This is copied from `helpers` to avoid a cycle: + _implicit_environ -> connection -> helpers -> key -> _implicit_environ + + :type key_pb: :class:`gcloud.datastore._datastore_v1_pb2.Key` + :param key_pb: A key to be added to a request. + + :rtype: :class:`gcloud.datastore._datastore_v1_pb2.Key` + :returns: A key which will be added to a request. It will be the + original if nothing needs to be changed. + """ + if key_pb.partition_id.HasField('dataset_id'): + new_key_pb = datastore_pb.Key() + new_key_pb.CopyFrom(key_pb) + new_key_pb.partition_id.ClearField('dataset_id') + key_pb = new_key_pb + return key_pb + + +def _add_keys_to_request(request_field_pb, key_pbs): + """Add protobuf keys to a request object. + + :type request_field_pb: `RepeatedCompositeFieldContainer` + :param request_field_pb: A repeated proto field that contains keys. + + :type key_pbs: list of :class:`gcloud.datastore._datastore_v1_pb2.Key` + :param key_pbs: The keys to add to a request. + """ + for key_pb in key_pbs: + key_pb = _prepare_key_for_request(key_pb) + request_field_pb.add().CopyFrom(key_pb) diff --git a/gcloud/datastore/helpers.py b/gcloud/datastore/helpers.py index 20b0b4fbe650..bb6f3fb0422e 100644 --- a/gcloud/datastore/helpers.py +++ b/gcloud/datastore/helpers.py @@ -308,17 +308,3 @@ def _prepare_key_for_request(key_pb): new_key_pb.partition_id.ClearField('dataset_id') key_pb = new_key_pb return key_pb - - -def _add_keys_to_request(request_field_pb, key_pbs): - """Add protobuf keys to a request object. - - :type request_field_pb: `RepeatedCompositeFieldContainer` - :param request_field_pb: A repeated proto field that contains keys. - - :type key_pbs: list of :class:`gcloud.datastore._datastore_v1_pb2.Key` - :param key_pbs: The keys to add to a request. - """ - for key_pb in key_pbs: - key_pb = _prepare_key_for_request(key_pb) - request_field_pb.add().CopyFrom(key_pb) From 7be6a1dc68b4af74faf4f5a6d22f123f5c6184d5 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Thu, 19 Feb 2015 13:03:05 -0800 Subject: [PATCH 2/4] Moving get_connection() from datastore.__init__ to _implicit_environ. --- gcloud/datastore/__init__.py | 30 ++-------------------- gcloud/datastore/_implicit_environ.py | 29 +++++++++++++++++++++ gcloud/datastore/test___init__.py | 20 --------------- gcloud/datastore/test__implicit_environ.py | 20 +++++++++++++++ 4 files changed, 51 insertions(+), 48 deletions(-) diff --git a/gcloud/datastore/__init__.py b/gcloud/datastore/__init__.py index ae91069998ea..d0906a22a2e5 100644 --- a/gcloud/datastore/__init__.py +++ b/gcloud/datastore/__init__.py @@ -46,8 +46,9 @@ when race conditions may occur. """ -from gcloud import credentials from gcloud.datastore import _implicit_environ +from gcloud.datastore._implicit_environ import SCOPE +from gcloud.datastore._implicit_environ import get_connection from gcloud.datastore._implicit_environ import get_default_connection from gcloud.datastore._implicit_environ import get_default_dataset_id from gcloud.datastore._implicit_environ import set_default_dataset_id @@ -63,11 +64,6 @@ from gcloud.datastore.transaction import Transaction -SCOPE = ('https://www.googleapis.com/auth/datastore', - 'https://www.googleapis.com/auth/userinfo.email') -"""The scopes required for authenticating as a Cloud Datastore consumer.""" - - def set_default_connection(connection=None): """Set default connection either explicitly or implicitly as fall-back. @@ -96,25 +92,3 @@ def set_defaults(dataset_id=None, connection=None): """ set_default_dataset_id(dataset_id=dataset_id) set_default_connection(connection=connection) - - -def get_connection(): - """Shortcut method to establish a connection to the Cloud Datastore. - - Use this if you are going to access several datasets - with the same set of credentials (unlikely): - - >>> from gcloud import datastore - - >>> connection = datastore.get_connection() - >>> key1 = datastore.Key('Kind', 1234, dataset_id='dataset1') - >>> key2 = datastore.Key('Kind', 1234, dataset_id='dataset2') - >>> entity1 = datastore.get(key1, connection=connection) - >>> entity2 = datastore.get(key2, connection=connection) - - :rtype: :class:`gcloud.datastore.connection.Connection` - :returns: A connection defined with the proper credentials. - """ - implicit_credentials = credentials.get_credentials() - scoped_credentials = implicit_credentials.create_scoped(SCOPE) - return Connection(credentials=scoped_credentials) diff --git a/gcloud/datastore/_implicit_environ.py b/gcloud/datastore/_implicit_environ.py index 8399d6b1775d..684e8b0afea4 100644 --- a/gcloud/datastore/_implicit_environ.py +++ b/gcloud/datastore/_implicit_environ.py @@ -28,6 +28,13 @@ except ImportError: app_identity = None +from gcloud import credentials +from gcloud.datastore.connection import Connection + + +SCOPE = ('https://www.googleapis.com/auth/datastore', + 'https://www.googleapis.com/auth/userinfo.email') +"""The scopes required for authenticating as a Cloud Datastore consumer.""" _DATASET_ENV_VAR_NAME = 'GCLOUD_DATASET_ID' _GCD_DATASET_ENV_VAR_NAME = 'DATASTORE_DATASET' @@ -143,6 +150,28 @@ def get_default_dataset_id(): return _DEFAULTS.dataset_id +def get_connection(): + """Shortcut method to establish a connection to the Cloud Datastore. + + Use this if you are going to access several datasets + with the same set of credentials (unlikely): + + >>> from gcloud import datastore + + >>> connection = datastore.get_connection() + >>> key1 = datastore.Key('Kind', 1234, dataset_id='dataset1') + >>> key2 = datastore.Key('Kind', 1234, dataset_id='dataset2') + >>> entity1 = datastore.get(key1, connection=connection) + >>> entity2 = datastore.get(key2, connection=connection) + + :rtype: :class:`gcloud.datastore.connection.Connection` + :returns: A connection defined with the proper credentials. + """ + implicit_credentials = credentials.get_credentials() + scoped_credentials = implicit_credentials.create_scoped(SCOPE) + return Connection(credentials=scoped_credentials) + + def get_default_connection(): """Get default connection. diff --git a/gcloud/datastore/test___init__.py b/gcloud/datastore/test___init__.py index 2b597162c9e3..380bc64e24c0 100644 --- a/gcloud/datastore/test___init__.py +++ b/gcloud/datastore/test___init__.py @@ -80,23 +80,3 @@ def call_set_connection(connection=None): self.assertEqual(SET_DATASET_CALLED, [DATASET_ID]) self.assertEqual(SET_CONNECTION_CALLED, [CONNECTION]) - - -class Test_get_connection(unittest2.TestCase): - - def _callFUT(self): - from gcloud.datastore import get_connection - return get_connection() - - def test_it(self): - from gcloud import credentials - from gcloud.datastore.connection import Connection - from gcloud.test_credentials import _Client - from gcloud._testing import _Monkey - - client = _Client() - with _Monkey(credentials, client=client): - found = self._callFUT() - self.assertTrue(isinstance(found, Connection)) - self.assertTrue(found._credentials is client._signed) - self.assertTrue(client._get_app_default_called) diff --git a/gcloud/datastore/test__implicit_environ.py b/gcloud/datastore/test__implicit_environ.py index b28db11d7a09..ff8d95e30e42 100644 --- a/gcloud/datastore/test__implicit_environ.py +++ b/gcloud/datastore/test__implicit_environ.py @@ -397,6 +397,26 @@ def mock_default(): 'dataset_id' in _implicit_environ._DEFAULTS.__dict__) +class Test_get_connection(unittest2.TestCase): + + def _callFUT(self): + from gcloud.datastore._implicit_environ import get_connection + return get_connection() + + def test_it(self): + from gcloud import credentials + from gcloud.datastore.connection import Connection + from gcloud.test_credentials import _Client + from gcloud._testing import _Monkey + + client = _Client() + with _Monkey(credentials, client=client): + found = self._callFUT() + self.assertTrue(isinstance(found, Connection)) + self.assertTrue(found._credentials is client._signed) + self.assertTrue(client._get_app_default_called) + + class _AppIdentity(object): def __init__(self, app_id): From d0fb4599200203798eb224a758b93966cd6d549c Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Thu, 19 Feb 2015 13:03:54 -0800 Subject: [PATCH 3/4] Moving set_default_connection() to _implicit_environ. Moved from datastore.__init__. --- gcloud/datastore/__init__.py | 12 +------- gcloud/datastore/_implicit_environ.py | 10 ++++++ gcloud/datastore/test___init__.py | 36 ---------------------- gcloud/datastore/test__implicit_environ.py | 35 +++++++++++++++++++++ 4 files changed, 46 insertions(+), 47 deletions(-) diff --git a/gcloud/datastore/__init__.py b/gcloud/datastore/__init__.py index d0906a22a2e5..98ce39854619 100644 --- a/gcloud/datastore/__init__.py +++ b/gcloud/datastore/__init__.py @@ -46,11 +46,11 @@ when race conditions may occur. """ -from gcloud.datastore import _implicit_environ from gcloud.datastore._implicit_environ import SCOPE from gcloud.datastore._implicit_environ import get_connection from gcloud.datastore._implicit_environ import get_default_connection from gcloud.datastore._implicit_environ import get_default_dataset_id +from gcloud.datastore._implicit_environ import set_default_connection from gcloud.datastore._implicit_environ import set_default_dataset_id from gcloud.datastore.api import allocate_ids from gcloud.datastore.api import delete @@ -64,16 +64,6 @@ from gcloud.datastore.transaction import Transaction -def set_default_connection(connection=None): - """Set default connection either explicitly or implicitly as fall-back. - - :type connection: :class:`gcloud.datastore.connection.Connection` - :param connection: A connection provided to be the default. - """ - connection = connection or get_connection() - _implicit_environ._DEFAULTS.connection = connection - - def set_defaults(dataset_id=None, connection=None): """Set defaults either explicitly or implicitly as fall-back. diff --git a/gcloud/datastore/_implicit_environ.py b/gcloud/datastore/_implicit_environ.py index 684e8b0afea4..83d7c3dc8857 100644 --- a/gcloud/datastore/_implicit_environ.py +++ b/gcloud/datastore/_implicit_environ.py @@ -172,6 +172,16 @@ def get_connection(): return Connection(credentials=scoped_credentials) +def set_default_connection(connection=None): + """Set default connection either explicitly or implicitly as fall-back. + + :type connection: :class:`gcloud.datastore.connection.Connection` + :param connection: A connection provided to be the default. + """ + connection = connection or get_connection() + _DEFAULTS.connection = connection + + def get_default_connection(): """Get default connection. diff --git a/gcloud/datastore/test___init__.py b/gcloud/datastore/test___init__.py index 380bc64e24c0..dd22d2787ad0 100644 --- a/gcloud/datastore/test___init__.py +++ b/gcloud/datastore/test___init__.py @@ -15,42 +15,6 @@ import unittest2 -class Test_set_default_connection(unittest2.TestCase): - - def setUp(self): - from gcloud.datastore._testing import _setup_defaults - _setup_defaults(self) - - def tearDown(self): - from gcloud.datastore._testing import _tear_down_defaults - _tear_down_defaults(self) - - def _callFUT(self, connection=None): - from gcloud.datastore import set_default_connection - return set_default_connection(connection=connection) - - def test_set_explicit(self): - from gcloud.datastore import _implicit_environ - - self.assertEqual(_implicit_environ.get_default_connection(), None) - fake_cnxn = object() - self._callFUT(connection=fake_cnxn) - self.assertEqual(_implicit_environ.get_default_connection(), fake_cnxn) - - def test_set_implicit(self): - from gcloud._testing import _Monkey - from gcloud import datastore - from gcloud.datastore import _implicit_environ - - self.assertEqual(_implicit_environ.get_default_connection(), None) - - fake_cnxn = object() - with _Monkey(datastore, get_connection=lambda: fake_cnxn): - self._callFUT() - - self.assertEqual(_implicit_environ.get_default_connection(), fake_cnxn) - - class Test_set_defaults(unittest2.TestCase): def _callFUT(self, dataset_id=None, connection=None): diff --git a/gcloud/datastore/test__implicit_environ.py b/gcloud/datastore/test__implicit_environ.py index ff8d95e30e42..0df9e276411d 100644 --- a/gcloud/datastore/test__implicit_environ.py +++ b/gcloud/datastore/test__implicit_environ.py @@ -417,6 +417,41 @@ def test_it(self): self.assertTrue(client._get_app_default_called) +class Test_set_default_connection(unittest2.TestCase): + + def setUp(self): + from gcloud.datastore._testing import _setup_defaults + _setup_defaults(self) + + def tearDown(self): + from gcloud.datastore._testing import _tear_down_defaults + _tear_down_defaults(self) + + def _callFUT(self, connection=None): + from gcloud.datastore._implicit_environ import set_default_connection + return set_default_connection(connection=connection) + + def test_set_explicit(self): + from gcloud.datastore import _implicit_environ + + self.assertEqual(_implicit_environ.get_default_connection(), None) + fake_cnxn = object() + self._callFUT(connection=fake_cnxn) + self.assertEqual(_implicit_environ.get_default_connection(), fake_cnxn) + + def test_set_implicit(self): + from gcloud._testing import _Monkey + from gcloud.datastore import _implicit_environ + + self.assertEqual(_implicit_environ.get_default_connection(), None) + + fake_cnxn = object() + with _Monkey(_implicit_environ, get_connection=lambda: fake_cnxn): + self._callFUT() + + self.assertEqual(_implicit_environ.get_default_connection(), fake_cnxn) + + class _AppIdentity(object): def __init__(self, app_id): From 7b0fc6e3d2929db9baf22ebbf145a0323f26ab9f Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Thu, 19 Feb 2015 13:08:27 -0800 Subject: [PATCH 4/4] Adding lazy loading support for datastore connection. --- gcloud/datastore/_implicit_environ.py | 9 +++++- gcloud/datastore/test__implicit_environ.py | 34 ++++++++++++---------- 2 files changed, 27 insertions(+), 16 deletions(-) diff --git a/gcloud/datastore/_implicit_environ.py b/gcloud/datastore/_implicit_environ.py index 83d7c3dc8857..3a4307865515 100644 --- a/gcloud/datastore/_implicit_environ.py +++ b/gcloud/datastore/_implicit_environ.py @@ -250,8 +250,15 @@ def dataset_id(): """Return the implicit default dataset ID.""" return _determine_default_dataset_id() + @_lazy_property_deco + @staticmethod + def connection(): + """Return the implicit default connection..""" + return get_connection() + def __init__(self, connection=None, dataset_id=None, implicit=False): - self.connection = connection + if connection is not None or not implicit: + self.connection = connection if dataset_id is not None or not implicit: self.dataset_id = dataset_id diff --git a/gcloud/datastore/test__implicit_environ.py b/gcloud/datastore/test__implicit_environ.py index 0df9e276411d..8fab07047d7d 100644 --- a/gcloud/datastore/test__implicit_environ.py +++ b/gcloud/datastore/test__implicit_environ.py @@ -344,7 +344,7 @@ def test_func(): self.assertEqual(lazy_prop._name, 'test_func') -class Test_lazy_loaded_dataset_id(unittest2.TestCase): +class Test_lazy_loading(unittest2.TestCase): def setUp(self): from gcloud.datastore._testing import _setup_defaults @@ -354,15 +354,6 @@ def tearDown(self): from gcloud.datastore._testing import _tear_down_defaults _tear_down_defaults(self) - def test_prop_default(self): - from gcloud.datastore import _implicit_environ - from gcloud.datastore._implicit_environ import _DefaultsContainer - from gcloud.datastore._implicit_environ import _LazyProperty - - self.assertTrue(isinstance(_DefaultsContainer.dataset_id, - _LazyProperty)) - self.assertEqual(_implicit_environ._DEFAULTS.dataset_id, None) - def test_prop_on_wrong_class(self): from gcloud.datastore._implicit_environ import _LazyProperty @@ -376,7 +367,7 @@ class FakeEnv(object): self.assertTrue(FakeEnv.dataset_id is data_prop) self.assertTrue(FakeEnv().dataset_id is data_prop) - def test_prop_descriptor(self): + def test_descriptor_for_dataset_id(self): from gcloud._testing import _Monkey from gcloud.datastore import _implicit_environ @@ -385,17 +376,30 @@ def test_prop_descriptor(self): DEFAULT = object() - def mock_default(): - return DEFAULT - with _Monkey(_implicit_environ, - _determine_default_dataset_id=mock_default): + _determine_default_dataset_id=lambda: DEFAULT): lazy_loaded = _implicit_environ._DEFAULTS.dataset_id self.assertEqual(lazy_loaded, DEFAULT) self.assertTrue( 'dataset_id' in _implicit_environ._DEFAULTS.__dict__) + def test_descriptor_for_connection(self): + from gcloud._testing import _Monkey + from gcloud.datastore import _implicit_environ + + self.assertFalse( + 'connection' in _implicit_environ._DEFAULTS.__dict__) + + DEFAULT = object() + + with _Monkey(_implicit_environ, get_connection=lambda: DEFAULT): + lazy_loaded = _implicit_environ._DEFAULTS.connection + + self.assertEqual(lazy_loaded, DEFAULT) + self.assertTrue( + 'connection' in _implicit_environ._DEFAULTS.__dict__) + class Test_get_connection(unittest2.TestCase):