DRY out read_options logic.

tseaver · tseaver · commit a2a11bd9fa96 · 2014-12-19T12:40:48.000-05:00
Feedback from @dhermes: #449 (comment) #449 (comment)
diff --git a/gcloud/datastore/connection.py b/gcloud/datastore/connection.py
@@ -233,17 +233,8 @@ def lookup(self, dataset_id, key_pbs,
         if deferred is not None and deferred != []:
             raise ValueError('deferred must be None or an empty list')
 
-        transaction = self.transaction()
-        if eventual and transaction:
-            raise ValueError('eventual must be False when in a transaction')
-
         lookup_request = datastore_pb.LookupRequest()
-
-        opts = lookup_request.read_options
-        if eventual:
-            opts.read_consistency = datastore_pb.ReadOptions.EVENTUAL
-        elif transaction:
-            opts.transaction = transaction
+        self._set_read_options(lookup_request, eventual)
 
         single_key = isinstance(key_pbs, datastore_pb.Key)
 
@@ -270,36 +261,6 @@ def lookup(self, dataset_id, key_pbs,
 
         return results
 
-    def _lookup(self, lookup_request, dataset_id, stop_on_deferred):
-        """Repeat lookup until all keys found (unless stop requested).
-
-        Helper method for ``lookup()``.
-        """
-        results = []
-        missing = []
-        deferred = []
-        while True:  # loop against possible deferred.
-            lookup_response = self._rpc(dataset_id, 'lookup', lookup_request,
-                                        datastore_pb.LookupResponse)
-
-            results.extend(
-                [result.entity for result in lookup_response.found])
-
-            missing.extend(
-                [result.entity for result in lookup_response.missing])
-
-            if stop_on_deferred:
-                deferred.extend([key for key in lookup_response.deferred])
-                break
-
-            if not lookup_response.deferred:
-                break
-
-            # We have deferred keys, and the user didn't ask to know about
-            # them, so retry (but only with the deferred ones).
-            _copy_deferred_keys(lookup_request, lookup_response)
-        return results, missing, deferred
-
     def run_query(self, dataset_id, query_pb, namespace=None, eventual=False):
         """Run a query on the Cloud Datastore.
 
@@ -351,16 +312,8 @@ def run_query(self, dataset_id, query_pb, namespace=None, eventual=False):
                         consistency.  If the connection has a current
                         transaction, this value *must* be false.
         """
-        transaction = self.transaction()
-        if eventual and transaction:
-            raise ValueError('eventual must be False when in a transaction')
-
         request = datastore_pb.RunQueryRequest()
-        opts = request.read_options
-        if eventual:
-            opts.read_consistency = datastore_pb.ReadOptions.EVENTUAL
-        elif transaction:
-            opts.transaction = transaction
+        self._set_read_options(request, eventual)
 
         if namespace:
             request.partition_id.namespace = namespace
@@ -563,6 +516,51 @@ def delete_entities(self, dataset_id, key_pbs):
 
         return True
 
+    def _lookup(self, lookup_request, dataset_id, stop_on_deferred):
+        """Repeat lookup until all keys found (unless stop requested).
+
+        Helper method for ``lookup()``.
+        """
+        results = []
+        missing = []
+        deferred = []
+        while True:  # loop against possible deferred.
+            lookup_response = self._rpc(dataset_id, 'lookup', lookup_request,
+                                        datastore_pb.LookupResponse)
+
+            results.extend(
+                [result.entity for result in lookup_response.found])
+
+            missing.extend(
+                [result.entity for result in lookup_response.missing])
+
+            if stop_on_deferred:
+                deferred.extend([key for key in lookup_response.deferred])
+                break
+
+            if not lookup_response.deferred:
+                break
+
+            # We have deferred keys, and the user didn't ask to know about
+            # them, so retry (but only with the deferred ones).
+            _copy_deferred_keys(lookup_request, lookup_response)
+        return results, missing, deferred
+
+    def _set_read_options(self, request, eventual):
+        """Validate rules for read options, and assign to the request.
+
+        Helper method for ``lookup()`` and ``run_query``.
+        """
+        transaction = self.transaction()
+        if eventual and transaction:
+            raise ValueError('eventual must be False when in a transaction')
+
+        opts = request.read_options
+        if eventual:
+            opts.read_consistency = datastore_pb.ReadOptions.EVENTUAL
+        elif transaction:
+            opts.transaction = transaction
+
 
 def _copy_deferred_keys(lookup_request, lookup_response):
     """Clear requested keys and copy deferred keys back in.
