From 3148468cd0341ffa21e7ec2c17b08c89b19dff10 Mon Sep 17 00:00:00 2001
From: Danny Hermes <daniel.j.hermes@gmail.com>
Date: Sun, 19 Oct 2014 18:23:40 -0700
Subject: [PATCH 1/2] Resolving cyclic imports in storage package.

This is accomplished by moving the definitions of
BucketIterator and KeyIterator into their respective
modules (i.e. storage.bucket and storage.key).

This is not so big an issue because the parent class
iterator.Iterator does most of the work and the children
only require a small tweak.
---
 gcloud/storage/bucket.py        | 27 +++++++++++-
 gcloud/storage/connection.py    |  2 +-
 gcloud/storage/iterator.py      | 54 -----------------------
 gcloud/storage/key.py           | 27 ++++++++++++
 gcloud/storage/test_bucket.py   | 40 ++++++++++++++++-
 gcloud/storage/test_iterator.py | 76 ---------------------------------
 gcloud/storage/test_key.py      | 40 +++++++++++++++++
 pylintrc_default                |  2 +-
 pylintrc_reduced                |  2 +-
 9 files changed, 134 insertions(+), 136 deletions(-)

diff --git a/gcloud/storage/bucket.py b/gcloud/storage/bucket.py
index f6c97ceef109..e9d26018949c 100644
--- a/gcloud/storage/bucket.py
+++ b/gcloud/storage/bucket.py
@@ -5,8 +5,9 @@
 from gcloud.storage import exceptions
 from gcloud.storage.acl import BucketACL
 from gcloud.storage.acl import DefaultObjectACL
-from gcloud.storage.iterator import KeyIterator
+from gcloud.storage.iterator import Iterator
 from gcloud.storage.key import Key
+from gcloud.storage.key import KeyIterator
 
 
 class Bucket(object):
@@ -637,3 +638,27 @@ def make_public(self, recursive=False, future=False):
             for key in self:
                 key.get_acl().all().grant_read()
                 key.save_acl()
+
+
+class BucketIterator(Iterator):
+    """An iterator listing all buckets.
+
+    You shouldn't have to use this directly,
+    but instead should use the helper methods
+    on :class:`gcloud.storage.connection.Connection` objects.
+
+    :type connection: :class:`gcloud.storage.connection.Connection`
+    :param connection: The connection to use for querying the list of buckets.
+    """
+
+    def __init__(self, connection):
+        super(BucketIterator, self).__init__(connection=connection, path='/b')
+
+    def get_items_from_response(self, response):
+        """Factory method which yields :class:`.Bucket` items from a response.
+
+        :type response: dict
+        :param response: The JSON API response for a page of buckets.
+        """
+        for item in response.get('items', []):
+            yield Bucket.from_dict(item, connection=self.connection)
diff --git a/gcloud/storage/connection.py b/gcloud/storage/connection.py
index 55b76ce3688d..9fd0b7d84b05 100644
--- a/gcloud/storage/connection.py
+++ b/gcloud/storage/connection.py
@@ -15,7 +15,7 @@
 from gcloud import connection
 from gcloud.storage import exceptions
 from gcloud.storage.bucket import Bucket
-from gcloud.storage.iterator import BucketIterator
+from gcloud.storage.bucket import BucketIterator
 
 
 def _utcnow():  # pragma: NO COVER testing replaces
diff --git a/gcloud/storage/iterator.py b/gcloud/storage/iterator.py
index 9bc01cb1eb56..144ac2fbabc5 100644
--- a/gcloud/storage/iterator.py
+++ b/gcloud/storage/iterator.py
@@ -134,60 +134,6 @@ def get_items_from_response(self, response):
         raise NotImplementedError
 
 
-class BucketIterator(Iterator):
-    """An iterator listing all buckets.
-
-    You shouldn't have to use this directly,
-    but instead should use the helper methods
-    on :class:`gcloud.storage.connection.Connection` objects.
-
-    :type connection: :class:`gcloud.storage.connection.Connection`
-    :param connection: The connection to use for querying the list of buckets.
-    """
-
-    def __init__(self, connection):
-        super(BucketIterator, self).__init__(connection=connection, path='/b')
-
-    def get_items_from_response(self, response):
-        """Factory method which yields :class:`.Bucket` items from a response.
-
-        :type response: dict
-        :param response: The JSON API response for a page of buckets.
-        """
-
-        from gcloud.storage.bucket import Bucket
-        for item in response.get('items', []):
-            yield Bucket.from_dict(item, connection=self.connection)
-
-
-class KeyIterator(Iterator):
-    """An iterator listing keys.
-
-    You shouldn't have to use this directly,
-    but instead should use the helper methods
-    on :class:`gcloud.storage.key.Key` objects.
-
-    :type bucket: :class:`gcloud.storage.bucket.Bucket`
-    :param bucket: The bucket from which to list keys.
-    """
-
-    def __init__(self, bucket):
-        self.bucket = bucket
-        super(KeyIterator, self).__init__(
-            connection=bucket.connection, path=bucket.path + '/o')
-
-    def get_items_from_response(self, response):
-        """Factory method, yields :class:`.storage.key.Key` items from response.
-
-        :type response: dict
-        :param response: The JSON API response for a page of keys.
-        """
-
-        from gcloud.storage.key import Key
-        for item in response.get('items', []):
-            yield Key.from_dict(item, bucket=self.bucket)
-
-
 class KeyDataIterator(object):
     """An iterator listing data stored in a key.
 
diff --git a/gcloud/storage/key.py b/gcloud/storage/key.py
index fa19a9afc05f..099c8ddbb62e 100644
--- a/gcloud/storage/key.py
+++ b/gcloud/storage/key.py
@@ -5,6 +5,7 @@
 from StringIO import StringIO
 
 from gcloud.storage.acl import ObjectACL
+from gcloud.storage.iterator import Iterator
 from gcloud.storage.iterator import KeyDataIterator
 
 
@@ -459,3 +460,29 @@ def make_public(self):
         self.get_acl().all().grant_read()
         self.save_acl()
         return self
+
+
+class KeyIterator(Iterator):
+    """An iterator listing keys.
+
+    You shouldn't have to use this directly,
+    but instead should use the helper methods
+    on :class:`gcloud.storage.key.Key` objects.
+
+    :type bucket: :class:`gcloud.storage.bucket.Bucket`
+    :param bucket: The bucket from which to list keys.
+    """
+
+    def __init__(self, bucket):
+        self.bucket = bucket
+        super(KeyIterator, self).__init__(
+            connection=bucket.connection, path=bucket.path + '/o')
+
+    def get_items_from_response(self, response):
+        """Factory method, yields :class:`.storage.key.Key` items from response.
+
+        :type response: dict
+        :param response: The JSON API response for a page of keys.
+        """
+        for item in response.get('items', []):
+            yield Key.from_dict(item, bucket=self.bucket)
diff --git a/gcloud/storage/test_bucket.py b/gcloud/storage/test_bucket.py
index f0ba26346121..8ae603a67cd0 100644
--- a/gcloud/storage/test_bucket.py
+++ b/gcloud/storage/test_bucket.py
@@ -840,7 +840,7 @@ def test_make_public_w_future(self):
     def test_make_public_recursive(self):
         from gcloud.storage.acl import _ACLEntity
         from gcloud._testing import _Monkey
-        from gcloud.storage import iterator
+        from gcloud.storage import key
         from gcloud.storage import bucket as MUT
         _saved = []
 
@@ -863,7 +863,7 @@ def grant_read(self):
             def save_acl(self):
                 _saved.append((self._bucket, self._name, self._granted))
 
-        class _KeyIterator(iterator.KeyIterator):
+        class _KeyIterator(key.KeyIterator):
             def get_items_from_response(self, response):
                 for item in response.get('items', []):
                     yield _Key(self.bucket, item['name'])
@@ -892,6 +892,42 @@ def get_items_from_response(self, response):
         self.assertEqual(kw[1]['query_params'], None)
 
 
+class TestBucketIterator(unittest2.TestCase):
+
+    def _getTargetClass(self):
+        from gcloud.storage.bucket import BucketIterator
+        return BucketIterator
+
+    def _makeOne(self, *args, **kw):
+        return self._getTargetClass()(*args, **kw)
+
+    def test_ctor(self):
+        connection = _Connection()
+        iterator = self._makeOne(connection)
+        self.assertTrue(iterator.connection is connection)
+        self.assertEqual(iterator.path, '/b')
+        self.assertEqual(iterator.page_number, 0)
+        self.assertEqual(iterator.next_page_token, None)
+
+    def test_get_items_from_response_empty(self):
+        connection = _Connection()
+        iterator = self._makeOne(connection)
+        self.assertEqual(list(iterator.get_items_from_response({})), [])
+
+    def test_get_items_from_response_non_empty(self):
+        from gcloud.storage.bucket import Bucket
+        KEY = 'key'
+        response = {'items': [{'name': KEY}]}
+        connection = _Connection()
+        iterator = self._makeOne(connection)
+        buckets = list(iterator.get_items_from_response(response))
+        self.assertEqual(len(buckets), 1)
+        bucket = buckets[0]
+        self.assertTrue(isinstance(bucket, Bucket))
+        self.assertTrue(bucket.connection is connection)
+        self.assertEqual(bucket.name, KEY)
+
+
 class _Connection(object):
     _delete_ok = False
 
diff --git a/gcloud/storage/test_iterator.py b/gcloud/storage/test_iterator.py
index 6dc1c9005bbc..8a0006ed0352 100644
--- a/gcloud/storage/test_iterator.py
+++ b/gcloud/storage/test_iterator.py
@@ -118,82 +118,6 @@ def test_get_items_from_response_raises_NotImplementedError(self):
                           iterator.get_items_from_response, object())
 
 
-class TestBucketIterator(unittest2.TestCase):
-
-    def _getTargetClass(self):
-        from gcloud.storage.iterator import BucketIterator
-        return BucketIterator
-
-    def _makeOne(self, *args, **kw):
-        return self._getTargetClass()(*args, **kw)
-
-    def test_ctor(self):
-        connection = _Connection()
-        iterator = self._makeOne(connection)
-        self.assertTrue(iterator.connection is connection)
-        self.assertEqual(iterator.path, '/b')
-        self.assertEqual(iterator.page_number, 0)
-        self.assertEqual(iterator.next_page_token, None)
-
-    def test_get_items_from_response_empty(self):
-        connection = _Connection()
-        iterator = self._makeOne(connection)
-        self.assertEqual(list(iterator.get_items_from_response({})), [])
-
-    def test_get_items_from_response_non_empty(self):
-        from gcloud.storage.bucket import Bucket
-        KEY = 'key'
-        response = {'items': [{'name': KEY}]}
-        connection = _Connection()
-        iterator = self._makeOne(connection)
-        buckets = list(iterator.get_items_from_response(response))
-        self.assertEqual(len(buckets), 1)
-        bucket = buckets[0]
-        self.assertTrue(isinstance(bucket, Bucket))
-        self.assertTrue(bucket.connection is connection)
-        self.assertEqual(bucket.name, KEY)
-
-
-class TestKeyIterator(unittest2.TestCase):
-
-    def _getTargetClass(self):
-        from gcloud.storage.iterator import KeyIterator
-        return KeyIterator
-
-    def _makeOne(self, *args, **kw):
-        return self._getTargetClass()(*args, **kw)
-
-    def test_ctor(self):
-        connection = _Connection()
-        bucket = _Bucket(connection)
-        iterator = self._makeOne(bucket)
-        self.assertTrue(iterator.bucket is bucket)
-        self.assertTrue(iterator.connection is connection)
-        self.assertEqual(iterator.path, '%s/o' % bucket.path)
-        self.assertEqual(iterator.page_number, 0)
-        self.assertEqual(iterator.next_page_token, None)
-
-    def test_get_items_from_response_empty(self):
-        connection = _Connection()
-        bucket = _Bucket(connection)
-        iterator = self._makeOne(bucket)
-        self.assertEqual(list(iterator.get_items_from_response({})), [])
-
-    def test_get_items_from_response_non_empty(self):
-        from gcloud.storage.key import Key
-        KEY = 'key'
-        response = {'items': [{'name': KEY}]}
-        connection = _Connection()
-        bucket = _Bucket(connection)
-        iterator = self._makeOne(bucket)
-        keys = list(iterator.get_items_from_response(response))
-        self.assertEqual(len(keys), 1)
-        key = keys[0]
-        self.assertTrue(isinstance(key, Key))
-        self.assertTrue(key.connection is connection)
-        self.assertEqual(key.name, KEY)
-
-
 class TestKeyDataIterator(unittest2.TestCase):
 
     def _getTargetClass(self):
diff --git a/gcloud/storage/test_key.py b/gcloud/storage/test_key.py
index 3a5028e1ebb7..9d02a16f8c9a 100644
--- a/gcloud/storage/test_key.py
+++ b/gcloud/storage/test_key.py
@@ -591,6 +591,46 @@ def test_make_public(self):
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
 
+class TestKeyIterator(unittest2.TestCase):
+
+    def _getTargetClass(self):
+        from gcloud.storage.key import KeyIterator
+        return KeyIterator
+
+    def _makeOne(self, *args, **kw):
+        return self._getTargetClass()(*args, **kw)
+
+    def test_ctor(self):
+        connection = _Connection()
+        bucket = _Bucket(connection)
+        iterator = self._makeOne(bucket)
+        self.assertTrue(iterator.bucket is bucket)
+        self.assertTrue(iterator.connection is connection)
+        self.assertEqual(iterator.path, '%s/o' % bucket.path)
+        self.assertEqual(iterator.page_number, 0)
+        self.assertEqual(iterator.next_page_token, None)
+
+    def test_get_items_from_response_empty(self):
+        connection = _Connection()
+        bucket = _Bucket(connection)
+        iterator = self._makeOne(bucket)
+        self.assertEqual(list(iterator.get_items_from_response({})), [])
+
+    def test_get_items_from_response_non_empty(self):
+        from gcloud.storage.key import Key
+        KEY = 'key'
+        response = {'items': [{'name': KEY}]}
+        connection = _Connection()
+        bucket = _Bucket(connection)
+        iterator = self._makeOne(bucket)
+        keys = list(iterator.get_items_from_response(response))
+        self.assertEqual(len(keys), 1)
+        key = keys[0]
+        self.assertTrue(isinstance(key, Key))
+        self.assertTrue(key.connection is connection)
+        self.assertEqual(key.name, KEY)
+
+
 class _Connection(object):
     API_BASE_URL = 'http://example.com'
 
diff --git a/pylintrc_default b/pylintrc_default
index e2c61f529e52..38d7b97eb2de 100644
--- a/pylintrc_default
+++ b/pylintrc_default
@@ -24,7 +24,7 @@ ignore = datastore_v1_pb2.py
 [MESSAGES CONTROL]
 disable = I, protected-access, maybe-no-member, no-member,
           redefined-builtin, star-args, missing-format-attribute,
-          similarities, cyclic-import, arguments-differ,
+          similarities, arguments-differ,
 
 
 
diff --git a/pylintrc_reduced b/pylintrc_reduced
index 3342430ab6f2..f8d3fbad384b 100644
--- a/pylintrc_reduced
+++ b/pylintrc_reduced
@@ -24,7 +24,7 @@ ignore = datastore_v1_pb2.py
 [MESSAGES CONTROL]
 disable = I, protected-access, maybe-no-member, no-member,
           redefined-builtin, star-args, missing-format-attribute,
-          similarities, cyclic-import, arguments-differ,
+          similarities, arguments-differ,
           invalid-name, missing-docstring, too-many-public-methods,
           too-few-public-methods, attribute-defined-outside-init,
           unbalanced-tuple-unpacking, too-many-locals, exec-used,

From 0a72dc4fe88d0df08309e91e737ac0275a599d09 Mon Sep 17 00:00:00 2001
From: Danny Hermes <daniel.j.hermes@gmail.com>
Date: Sun, 19 Oct 2014 20:45:50 -0700
Subject: [PATCH 2/2] 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.
---
 gcloud/datastore/_dataset.py      | 17 +++++++++++
 gcloud/datastore/_helpers.py      | 22 ++++++++-------
 gcloud/datastore/connection.py    |  4 ++-
 gcloud/datastore/dataset.py       | 47 +++++--------------------------
 gcloud/datastore/entity.py        |  8 ++----
 gcloud/datastore/key.py           |  4 +--
 gcloud/datastore/query.py         |  3 +-
 gcloud/datastore/test__helpers.py | 21 +++++++-------
 gcloud/storage/test_iterator.py   |  7 -----
 9 files changed, 57 insertions(+), 76 deletions(-)
 create mode 100644 gcloud/datastore/_dataset.py

diff --git a/gcloud/datastore/_dataset.py b/gcloud/datastore/_dataset.py
new file mode 100644
index 000000000000..e0a7e75b5cca
--- /dev/null
+++ b/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
diff --git a/gcloud/datastore/_helpers.py b/gcloud/datastore/_helpers.py
index 908c6d79345e..b42ec72f0e2e 100644
--- a/gcloud/datastore/_helpers.py
+++ b/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':
diff --git a/gcloud/datastore/connection.py b/gcloud/datastore/connection.py
index aff322d90926..3fb8d0df1da4 100644
--- a/gcloud/datastore/connection.py
+++ b/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.
diff --git a/gcloud/datastore/dataset.py b/gcloud/datastore/dataset.py
index 8ec22035c929..8dbb4a507bb6 100644
--- a/gcloud/datastore/dataset.py
+++ b/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]
diff --git a/gcloud/datastore/entity.py b/gcloud/datastore/entity.py
index e992a8018379..3f853f7a5ab3 100644
--- a/gcloud/datastore/entity.py
+++ b/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
diff --git a/gcloud/datastore/key.py b/gcloud/datastore/key.py
index ccc89c2360c9..13b8149a98cc 100644
--- a/gcloud/datastore/key.py
+++ b/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
diff --git a/gcloud/datastore/query.py b/gcloud/datastore/query.py
index fa1961864340..ee266214cc1d 100644
--- a/gcloud/datastore/query.py
+++ b/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):
diff --git a/gcloud/datastore/test__helpers.py b/gcloud/datastore/test__helpers.py
index 3fbfb38e08c4..f2b5453b4945 100644
--- a/gcloud/datastore/test__helpers.py
+++ b/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)
diff --git a/gcloud/storage/test_iterator.py b/gcloud/storage/test_iterator.py
index 8a0006ed0352..8d441b1dce5c 100644
--- a/gcloud/storage/test_iterator.py
+++ b/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'