Merge pull request #352 from tseaver/351-batch_updates_via_context_manager

Fix #351: batch updates via context manager

Author: tseaver

gcloud/storage/_helpers.py

@@ -55,6 +55,30 @@ def properties(self):
             self._reload_properties()
         return self._properties.copy()
 
+    @property
+    def batch(self):
+        """Return a context manager which defers/batches updates.
+
+        E.g., to batch multiple updates to a bucket::
+
+            >>> with bucket.batch:
+            ...     bucket.enable_versioning()
+            ...     bucket.disable_website()
+
+        or for a key::
+
+            >>> with key.batch:
+            ...     key.content_type = 'image/jpeg'
+            ...     key.content_encoding = 'gzip'
+
+        Updates will be aggregated and sent as a single call to
+        :meth:`_patch_properties` IFF the ``with`` block exits without
+        an exception.
+
+        :rtype: :class:`_PropertyBatch`
+        """
+        return _PropertyBatch(self)
+
     def _reload_properties(self):
         """Reload properties from Cloud Storage.
 
@@ -122,3 +146,25 @@ def get_acl(self):
         if not self.acl.loaded:
             self.acl.reload()
         return self.acl
+
+
+class _PropertyBatch(object):
+    """Context manager: Batch updates to object's ``_patch_properties``
+
+    :type wrapped: class derived from :class:`_PropertyMixin`.
+    :param wrapped:  the instance whose property updates to defer/batch.
+    """
+    def __init__(self, wrapped):
+        self._wrapped = wrapped
+        self._deferred = {}
+
+    def __enter__(self):
+        """Intercept / defer property updates."""
+        self._wrapped._patch_properties = self._deferred.update
+
+    def __exit__(self, type, value, traceback):
+        """Patch deferred property updates if no error."""
+        del self._wrapped._patch_properties
+        if type is None:
+            if self._deferred:
+                self._wrapped._patch_properties(self._deferred)

gcloud/storage/bucket.py

@@ -544,7 +544,7 @@ def storage_class(self):
         """Retrieve the storage class for the bucket.
 
         See: https://cloud.google.com/storage/docs/json_api/v1/buckets and
-        https://cloud.google.com/storage/docs/durable-reduced-availability#_DRA_Bucket
+        https://cloud.google.com/storage/docs/durable-reduced-availability
 
         :rtype: string
         :returns: the storage class for the bucket (currently one of

gcloud/storage/key.py

@@ -376,7 +376,7 @@ def __init__(self, bucket, extra_params=None):
             extra_params=extra_params)
 
     def get_items_from_response(self, response):
-        """Factory method, yields :class:`.storage.key.Key` items from response.
+        """Yield :class:`.storage.key.Key` items from response.
 
         :type response: dict
         :param response: The JSON API response for a page of keys.

gcloud/storage/test__helpers.py

@@ -33,6 +33,34 @@ def test_path_is_abstract(self):
         mixin = self._makeOne()
         self.assertRaises(NotImplementedError, lambda: mixin.path)
 
+    def test_properties_eager(self):
+        derived = self._derivedClass()(properties={'extant': False})
+        self.assertEqual(derived.properties, {'extant': False})
+
+    def test_batch(self):
+        connection = _Connection({'foo': 'Qux', 'bar': 'Baz'})
+        derived = self._derivedClass(connection, '/path')()
+        with derived.batch:
+            derived._patch_properties({'foo': 'Foo'})
+            derived._patch_properties({'bar': 'Baz'})
+            derived._patch_properties({'foo': 'Qux'})
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'PATCH')
+        self.assertEqual(kw[0]['path'], '/path')
+        self.assertEqual(kw[0]['data'], {'foo': 'Qux', 'bar': 'Baz'})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+
+    def test_properties_lazy(self):
+        connection = _Connection({'foo': 'Foo'})
+        derived = self._derivedClass(connection, '/path')()
+        self.assertEqual(derived.properties, {'foo': 'Foo'})
+        kw = connection._requested
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'GET')
+        self.assertEqual(kw[0]['path'], '/path')
+        self.assertEqual(kw[0]['query_params'], {'projection': 'noAcl'})
+
     def test__reload_properties(self):
         connection = _Connection({'foo': 'Foo'})
         derived = self._derivedClass(connection, '/path')()
@@ -89,20 +117,6 @@ def test__patch_properties(self):
         self.assertEqual(kw[0]['data'], {'foo': 'Foo'})
         self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
 
-    def test_properties_eager(self):
-        derived = self._derivedClass()(properties={'extant': False})
-        self.assertEqual(derived.properties, {'extant': False})
-
-    def test_properties_lazy(self):
-        connection = _Connection({'foo': 'Foo'})
-        derived = self._derivedClass(connection, '/path')()
-        self.assertEqual(derived.properties, {'foo': 'Foo'})
-        kw = connection._requested
-        self.assertEqual(len(kw), 1)
-        self.assertEqual(kw[0]['method'], 'GET')
-        self.assertEqual(kw[0]['path'], '/path')
-        self.assertEqual(kw[0]['query_params'], {'projection': 'noAcl'})
-
     def test_get_acl_not_yet_loaded(self):
         class ACL(object):
             loaded = False
@@ -123,6 +137,83 @@ class ACL(object):
         self.assertTrue(mixin.get_acl() is acl)  # no 'reload'
 
 
+class TestPropertyBatch(unittest2.TestCase):
+
+    def _getTargetClass(self):
+        from gcloud.storage._helpers import _PropertyBatch
+        return _PropertyBatch
+
+    def _makeOne(self, wrapped):
+        return self._getTargetClass()(wrapped)
+
+    def _makeWrapped(self, connection=None, path=None, **custom_fields):
+        from gcloud.storage._helpers import _PropertyMixin
+
+        class Wrapped(_PropertyMixin):
+            CUSTOM_PROPERTY_ACCESSORS = custom_fields
+
+            @property
+            def connection(self):
+                return connection
+
+            @property
+            def path(self):
+                return path
+
+        return Wrapped()
+
+    def test_ctor_does_not_intercept__patch_properties(self):
+        wrapped = self._makeWrapped()
+        before = wrapped._patch_properties
+        batch = self._makeOne(wrapped)
+        after = wrapped._patch_properties
+        self.assertEqual(before, after)
+        self.assertTrue(batch._wrapped is wrapped)
+
+    def test_cm_intercepts_restores__patch_properties(self):
+        wrapped = self._makeWrapped()
+        before = wrapped._patch_properties
+        batch = self._makeOne(wrapped)
+        with batch:
+            # No deferred patching -> no call to the real '_patch_properties'
+            during = wrapped._patch_properties
+        after = wrapped._patch_properties
+        self.assertNotEqual(before, during)
+        self.assertEqual(before, after)
+
+    def test___exit___w_error_skips__patch_properties(self):
+        class Testing(Exception):
+            pass
+        wrapped = self._makeWrapped()
+        batch = self._makeOne(wrapped)
+        try:
+            with batch:
+                # deferred patching
+                wrapped._patch_properties({'foo': 'Foo'})
+                # but error -> no call to the real '_patch_properties'
+                raise Testing('testing')
+        except Testing:
+            pass
+
+    def test___exit___no_error_aggregates__patch_properties(self):
+        connection = _Connection({'foo': 'Foo'})
+        wrapped = self._makeWrapped(connection, '/path')
+        batch = self._makeOne(wrapped)
+        kw = connection._requested
+        with batch:
+            # deferred patching
+            wrapped._patch_properties({'foo': 'Foo'})
+            wrapped._patch_properties({'bar': 'Baz'})
+            wrapped._patch_properties({'foo': 'Qux'})
+            self.assertEqual(len(kw), 0)
+        # exited w/o error -> call to the real '_patch_properties'
+        self.assertEqual(len(kw), 1)
+        self.assertEqual(kw[0]['method'], 'PATCH')
+        self.assertEqual(kw[0]['path'], '/path')
+        self.assertEqual(kw[0]['data'], {'foo': 'Qux', 'bar': 'Baz'})
+        self.assertEqual(kw[0]['query_params'], {'projection': 'full'})
+
+
 class _Connection(object):
 
     def __init__(self, *responses):

pylintrc_default

@@ -7,7 +7,6 @@ good-names = i, j, k, ex, Run, _, pb, id,
 
 [DESIGN]
 max-args = 10
-max-public-methods = 40
 
 [FORMAT]
 # NOTE: By default pylint ignores whitespace checks around the
@@ -24,7 +23,8 @@ ignore = datastore_v1_pb2.py
 [MESSAGES CONTROL]
 disable = I, protected-access, maybe-no-member, no-member,
           redefined-builtin, star-args, missing-format-attribute,
-          similarities, arguments-differ
+          similarities, arguments-differ,
+          too-many-public-methods, too-few-public-methods
 
 [REPORTS]
 reports = no