Temp commit with idea to clear up cyclic imports in datastore package.

This is intended for discussion but not to be committed. Some remarks:

- This surfaces the fact that the use of Dataset in
  datastore.key.Key.from_protobuf is not well-tested enough.
- The changes to _helpers are made to avoid explicitly referencing
  the Entity class. These changes end up in uglier code
  for a minimal gain.
- We could likely factor out the Dataset ID/connection concept from
  the full "dataset" concept. It seems a Dataset and its convenience
  methods are referenced a lot in the docs but not really tested
  much.

Author: dhermes

gcloud/datastore/_dataset.py

@@ -0,0 +1,17 @@
+"""Needs module docstring."""
+
+
+class _Dataset(object):
+    """Needs class docstring."""
+
+    def __init__(self, id, connection=None):
+        self._connection = connection
+        self._id = id
+
+    def connection(self):
+        """Needs method docstring."""
+        return self._connection
+
+    def id(self):
+        """Needs method docstring."""
+        return self._id

gcloud/datastore/_helpers.py

@@ -8,13 +8,12 @@
 from google.protobuf.internal.type_checkers import Int64ValueChecker
 import pytz
 
-from gcloud.datastore.entity import Entity
 from gcloud.datastore.key import Key
 
 INT_VALUE_CHECKER = Int64ValueChecker()
 
 
-def _get_protobuf_attribute_and_value(val):
+def _get_protobuf_attribute_and_value(val, entity_class=type(None)):
     """Given a value, return the protobuf attribute name and proper value.
 
     The Protobuf API uses different attribute names
@@ -64,7 +63,7 @@ def _get_protobuf_attribute_and_value(val):
         name, value = 'integer', long(val)  # Always cast to a long.
     elif isinstance(val, basestring):
         name, value = 'string', val
-    elif isinstance(val, Entity):
+    elif isinstance(val, entity_class):
         name, value = 'entity', val
     elif isinstance(val, list):
         name, value = 'list', val
@@ -74,7 +73,7 @@ def _get_protobuf_attribute_and_value(val):
     return name + '_value', value
 
 
-def _get_value_from_value_pb(value_pb):
+def _get_value_from_value_pb(value_pb, entity_class=type(None)):
     """Given a protobuf for a Value, get the correct value.
 
     The Cloud Datastore Protobuf API returns a Property Protobuf
@@ -113,15 +112,16 @@ def _get_value_from_value_pb(value_pb):
         result = value_pb.string_value
 
     elif value_pb.HasField('entity_value'):
-        result = Entity.from_protobuf(value_pb.entity_value)
+        result = entity_class.from_protobuf(value_pb.entity_value)
 
     elif value_pb.list_value:
-        result = [_get_value_from_value_pb(x) for x in value_pb.list_value]
+        result = [_get_value_from_value_pb(x, entity_class=entity_class)
+                  for x in value_pb.list_value]
 
     return result
 
 
-def _get_value_from_property_pb(property_pb):
+def _get_value_from_property_pb(property_pb, entity_class=type(None)):
     """Given a protobuf for a Property, get the correct value.
 
     The Cloud Datastore Protobuf API returns a Property Protobuf
@@ -136,10 +136,11 @@ def _get_value_from_property_pb(property_pb):
 
     :returns: The value provided by the Protobuf.
     """
-    return _get_value_from_value_pb(property_pb.value)
+    return _get_value_from_value_pb(property_pb.value,
+                                    entity_class=entity_class)
 
 
-def _set_protobuf_value(value_pb, val):
+def _set_protobuf_value(value_pb, val, entity_class=type(None)):
     """Assign 'val' to the correct subfield of 'value_pb'.
 
     The Protobuf API uses different attribute names
@@ -156,7 +157,8 @@ def _set_protobuf_value(value_pb, val):
                :class:`gcloud.datastore.entity.Entity`,
     :param val: The value to be assigned.
     """
-    attr, val = _get_protobuf_attribute_and_value(val)
+    attr, val = _get_protobuf_attribute_and_value(val,
+                                                  entity_class=entity_class)
     if attr == 'key_value':
         value_pb.key_value.CopyFrom(val)
     elif attr == 'entity_value':

gcloud/datastore/connection.py

@@ -4,6 +4,7 @@
 from gcloud.datastore import datastore_v1_pb2 as datastore_pb
 from gcloud.datastore import _helpers
 from gcloud.datastore.dataset import Dataset
+from gcloud.datastore.entity import Entity
 
 
 class Connection(connection.Connection):
@@ -374,7 +375,8 @@ def save_entity(self, dataset_id, key_pb, properties):
             prop.name = name
 
             # Set the appropriate value.
-            _helpers._set_protobuf_value(prop.value, value)
+            _helpers._set_protobuf_value(prop.value, value,
+                                         entity_class=Entity)
 
         # If this is in a transaction, we should just return True. The
         # transaction will handle assigning any keys as necessary.

gcloud/datastore/dataset.py

@@ -1,7 +1,13 @@
 """Create / interact with gcloud datastore datasets."""
 
 
-class Dataset(object):
+from gcloud.datastore._dataset import _Dataset
+from gcloud.datastore.entity import Entity
+from gcloud.datastore.query import Query
+from gcloud.datastore.transaction import Transaction
+
+
+class Dataset(_Dataset):
     """A dataset in the Cloud Datastore.
 
     This class acts as an abstraction of a single dataset
@@ -30,36 +36,6 @@ class Dataset(object):
     :param connection: The connection to use for executing API calls.
     """
 
-    def __init__(self, id, connection=None):
-        self._connection = connection
-        self._id = id
-
-    def connection(self):
-        """Get the current connection.
-
-          >>> dataset = Dataset('dataset-id', connection=conn)
-          >>> dataset.connection()
-          <Connection object>
-
-        :rtype: :class:`gcloud.datastore.connection.Connection`
-        :returns: Returns the current connection.
-        """
-
-        return self._connection
-
-    def id(self):
-        """Get the current dataset ID.
-
-          >>> dataset = Dataset('dataset-id', connection=conn)
-          >>> dataset.id()
-          'dataset-id'
-
-        :rtype: string
-        :returns: The current dataset ID.
-        """
-
-        return self._id
-
     def query(self, *args, **kwargs):
         """Create a query bound to this dataset.
 
@@ -70,8 +46,6 @@ def query(self, *args, **kwargs):
         :rtype: :class:`gcloud.datastore.query.Query`
         :returns: a new Query instance, bound to this dataset.
         """
-        # This import is here to avoid circular references.
-        from gcloud.datastore.query import Query
         kwargs['dataset'] = self
         return Query(*args, **kwargs)
 
@@ -84,8 +58,6 @@ def entity(self, kind):
         :rtype: :class:`gcloud.datastore.entity.Entity`
         :returns: a new Entity instance, bound to this dataset.
         """
-        # This import is here to avoid circular references.
-        from gcloud.datastore.entity import Entity
         return Entity(dataset=self, kind=kind)
 
     def transaction(self, *args, **kwargs):
@@ -98,8 +70,6 @@ def transaction(self, *args, **kwargs):
         :rtype: :class:`gcloud.datastore.transaction.Transaction`
         :returns: a new Transaction instance, bound to this dataset.
         """
-        # This import is here to avoid circular references.
-        from gcloud.datastore.transaction import Transaction
         kwargs['dataset'] = self
         return Transaction(*args, **kwargs)
 
@@ -125,9 +95,6 @@ def get_entities(self, keys):
         :rtype: list of :class:`gcloud.datastore.entity.Entity`
         :return: The requested entities.
         """
-        # This import is here to avoid circular references.
-        from gcloud.datastore.entity import Entity
-
         entity_pbs = self.connection().lookup(
             dataset_id=self.id(),
             key_pbs=[k.to_protobuf() for k in keys]

gcloud/datastore/entity.py

@@ -15,6 +15,7 @@
 delete or persist the data stored on the entity.
 """
 
+from gcloud.datastore import _helpers
 from gcloud.datastore import datastore_v1_pb2 as datastore_pb
 from gcloud.datastore.key import Key
 
@@ -155,15 +156,12 @@ def from_protobuf(cls, pb, dataset=None):
         :returns: The :class:`Entity` derived from the
                   :class:`gcloud.datastore.datastore_v1_pb2.Entity`.
         """
-
-        # This is here to avoid circular imports.
-        from gcloud.datastore import _helpers
-
         key = Key.from_protobuf(pb.key, dataset=dataset)
         entity = cls.from_key(key)
 
         for property_pb in pb.property:
-            value = _helpers._get_value_from_property_pb(property_pb)
+            value = _helpers._get_value_from_property_pb(property_pb,
+                                                         entity_class=Entity)
             entity[property_pb.name] = value
 
         return entity

gcloud/datastore/key.py

@@ -3,8 +3,8 @@
 import copy
 from itertools import izip
 
+from gcloud.datastore._dataset import _Dataset
 from gcloud.datastore import datastore_v1_pb2 as datastore_pb
-from gcloud.datastore.dataset import Dataset
 
 
 class Key(object):
@@ -76,7 +76,7 @@ def from_protobuf(cls, pb, dataset=None):
             path.append(element_dict)
 
         if not dataset:
-            dataset = Dataset(id=pb.partition_id.dataset_id)
+            dataset = _Dataset(id=pb.partition_id.dataset_id)
             namespace = pb.partition_id.namespace
         else:
             namespace = None

gcloud/datastore/query.py

@@ -138,7 +138,8 @@ def filter(self, expression, value):
         property_filter.operator = operator
 
         # Set the value to filter on based on the type.
-        _helpers._set_protobuf_value(property_filter.value, value)
+        _helpers._set_protobuf_value(property_filter.value, value,
+                                     entity_class=Entity)
         return clone
 
     def ancestor(self, ancestor):

gcloud/datastore/test__helpers.py

@@ -3,10 +3,11 @@
 
 class Test__get_protobuf_attribute_and_value(unittest2.TestCase):
 
-    def _callFUT(self, val):
+    def _callFUT(self, val, entity_class=type(None)):
         from gcloud.datastore._helpers import _get_protobuf_attribute_and_value
 
-        return _get_protobuf_attribute_and_value(val)
+        return _get_protobuf_attribute_and_value(val,
+                                                 entity_class=entity_class)
 
     def test_datetime_naive(self):
         import calendar
@@ -86,7 +87,7 @@ def test_unicode(self):
     def test_entity(self):
         from gcloud.datastore.entity import Entity
         entity = Entity()
-        name, value = self._callFUT(entity)
+        name, value = self._callFUT(entity, entity_class=Entity)
         self.assertEqual(name, 'entity_value')
         self.assertTrue(value is entity)
 
@@ -102,10 +103,10 @@ def test_object(self):
 
 class Test__get_value_from_value_pb(unittest2.TestCase):
 
-    def _callFUT(self, pb):
+    def _callFUT(self, pb, entity_class=type(None)):
         from gcloud.datastore._helpers import _get_value_from_value_pb
 
-        return _get_value_from_value_pb(pb)
+        return _get_value_from_value_pb(pb, entity_class=entity_class)
 
     def _makePB(self, attr_name, value):
         from gcloud.datastore.datastore_v1_pb2 import Value
@@ -168,7 +169,7 @@ def test_entity(self):
         prop_pb = entity_pb.property.add()
         prop_pb.name = 'foo'
         prop_pb.value.string_value = 'Foo'
-        entity = self._callFUT(pb)
+        entity = self._callFUT(pb, entity_class=Entity)
         self.assertTrue(isinstance(entity, Entity))
         self.assertEqual(entity['foo'], 'Foo')
 
@@ -208,10 +209,10 @@ def test_it(self):
 
 class Test_set_protobuf_value(unittest2.TestCase):
 
-    def _callFUT(self, value_pb, val):
+    def _callFUT(self, value_pb, val, entity_class=type(None)):
         from gcloud.datastore._helpers import _set_protobuf_value
 
-        return _set_protobuf_value(value_pb, val)
+        return _set_protobuf_value(value_pb, val, entity_class=entity_class)
 
     def _makePB(self):
         from gcloud.datastore.datastore_v1_pb2 import Value
@@ -286,7 +287,7 @@ def test_entity_empty_wo_key(self):
 
         pb = self._makePB()
         entity = Entity()
-        self._callFUT(pb, entity)
+        self._callFUT(pb, entity, entity_class=Entity)
         value = pb.entity_value
         self.assertEqual(value.key.SerializeToString(), '')
         props = list(value.property)
@@ -300,7 +301,7 @@ def test_entity_w_key(self):
         key = Key(path=[{'kind': 'KIND', 'id': 123}])
         entity = Entity().key(key)
         entity['foo'] = 'Foo'
-        self._callFUT(pb, entity)
+        self._callFUT(pb, entity, entity_class=Entity)
         value = pb.entity_value
         self.assertEqual(value.key, key.to_protobuf())
         props = list(value.property)

gcloud/storage/test_iterator.py

@@ -313,13 +313,6 @@ def build_api_url(self, path, query_params=None):
         return urlunsplit(('http', 'example.com', path, qs, ''))
 
 
-class _Bucket(object):
-    path = '/b/name'
-
-    def __init__(self, connection):
-        self.connection = connection
-
-
 class _Key(object):
     CHUNK_SIZE = 10
     path = '/b/name/o/key'