Merge pull request #783 from dhermes/fix-776

Remove uses of 'return self' that don't return copies.

gcloud/connection.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-""" Shared implementation of connections to API servers."""
+"""Shared implementation of connections to API servers."""
 
 import json
 from pkg_resources import get_distribution

gcloud/datastore/batch.py

@@ -57,7 +57,7 @@ class Batch(object):
     """
 
     def __init__(self, dataset_id=None, connection=None):
-        """ Construct a batch.
+        """Construct a batch.
 
         :type dataset_id: :class:`str`.
         :param dataset_id: The ID of the dataset.

gcloud/pubsub/api.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-""" Define API functions (not bound to classes)."""
+"""Define API functions (not bound to classes)."""
 
 from gcloud._helpers import get_default_project
 from gcloud.pubsub._implicit_environ import get_default_connection

gcloud/pubsub/subscription.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-""" Define API Subscriptions."""
+"""Define API Subscriptions."""
 
 from gcloud.exceptions import NotFound
 

gcloud/pubsub/topic.py

@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-""" Define API Topics."""
+"""Define API Topics."""
 
 import base64
 

gcloud/storage/_helpers.py

@@ -50,17 +50,12 @@ def __init__(self, name=None):
         self._changes = set()
 
     def reload(self):
-        """Reload properties from Cloud Storage.
-
-        :rtype: :class:`_PropertyMixin`
-        :returns: The object you just reloaded data for.
-        """
+        """Reload properties from Cloud Storage."""
         # Pass only '?projection=noAcl' here because 'acl' and related
         # are handled via custom endpoints.
         query_params = {'projection': 'noAcl'}
         self._properties = self.connection.api_request(
             method='GET', path=self.path, query_params=query_params)
-        return self
 
     def _patch_properties(self, properties):
         """Update particular fields of this object's properties.
@@ -73,21 +68,14 @@ def _patch_properties(self, properties):
 
         :type properties: dict
         :param properties: The dictionary of values to update.
-
-        :rtype: :class:`_PropertyMixin`
-        :returns: The current object.
         """
         self._changes.update(properties.keys())
         self._properties.update(properties)
-        return self
 
     def patch(self):
         """Sends all changed properties in a PATCH request.
 
-        Updates the ``properties`` with the response from the backend.
-
-        :rtype: :class:`Bucket`
-        :returns: The current bucket.
+        Updates the ``_properties`` with the response from the backend.
         """
         # Pass '?projection=full' here because 'PATCH' documented not
         # to work properly w/ 'noAcl'.
@@ -96,7 +84,6 @@ def patch(self):
         self._properties = self.connection.api_request(
             method='PATCH', path=self.path, data=update_properties,
             query_params={'projection': 'full'})
-        return self
 
 
 def _scalar_property(fieldname):

gcloud/storage/acl.py

@@ -127,55 +127,41 @@ def grant(self, role):
 
         :type role: string
         :param role: The role to add to the entity.
-
-        :rtype: :class:`_ACLEntity`
-        :returns: The entity class.
         """
         self.roles.add(role)
-        return self
 
     def revoke(self, role):
         """Remove a role from the entity.
 
         :type role: string
         :param role: The role to remove from the entity.
-
-        :rtype: :class:`_ACLEntity`
-        :returns: The entity class.
         """
         if role in self.roles:
             self.roles.remove(role)
-        return self
 
     def grant_read(self):
         """Grant read access to the current entity."""
-
-        return self.grant(_ACLEntity.READER_ROLE)
+        self.grant(_ACLEntity.READER_ROLE)
 
     def grant_write(self):
         """Grant write access to the current entity."""
-
-        return self.grant(_ACLEntity.WRITER_ROLE)
+        self.grant(_ACLEntity.WRITER_ROLE)
 
     def grant_owner(self):
         """Grant owner access to the current entity."""
-
-        return self.grant(_ACLEntity.OWNER_ROLE)
+        self.grant(_ACLEntity.OWNER_ROLE)
 
     def revoke_read(self):
         """Revoke read access from the current entity."""
-
-        return self.revoke(_ACLEntity.READER_ROLE)
+        self.revoke(_ACLEntity.READER_ROLE)
 
     def revoke_write(self):
         """Revoke write access from the current entity."""
-
-        return self.revoke(_ACLEntity.WRITER_ROLE)
+        self.revoke(_ACLEntity.WRITER_ROLE)
 
     def revoke_owner(self):
         """Revoke owner access from the current entity."""
-
-        return self.revoke(_ACLEntity.OWNER_ROLE)
+        self.revoke(_ACLEntity.OWNER_ROLE)
 
 
 class ACL(object):
@@ -234,7 +220,8 @@ def entity_from_dict(self, entity_dict):
         if not isinstance(entity, _ACLEntity):
             raise ValueError('Invalid dictionary: %s' % entity_dict)
 
-        return entity.grant(role)
+        entity.grant(role)
+        return entity
 
     def has_entity(self, entity):
         """Returns whether or not this ACL has any entries for an entity.
@@ -361,8 +348,9 @@ def get_entities(self):
     def reload(self):
         """Reload the ACL data from Cloud Storage.
 
-        :rtype: :class:`ACL`
-        :returns: The current ACL.
+        This is a virtual method, expected to be implemented by subclasses.
+
+        :raises: :class:`NotImplementedError`
         """
         raise NotImplementedError
 
@@ -396,11 +384,7 @@ def __init__(self, bucket):
         self.bucket = bucket
 
     def reload(self):
-        """Reload the ACL data from Cloud Storage.
-
-        :rtype: :class:`gcloud.storage.acl.BucketACL`
-        :returns: The current ACL.
-        """
+        """Reload the ACL data from Cloud Storage."""
         self.entities.clear()
 
         url_path = '%s/%s' % (self.bucket.path, self._URL_PATH_ELEM)
@@ -409,8 +393,6 @@ def reload(self):
         for entry in found.get('items', ()):
             self.add_entity(self.entity_from_dict(entry))
 
-        return self
-
     def save(self, acl=None):
         """Save this ACL for the current bucket.
 
@@ -434,9 +416,6 @@ def save(self, acl=None):
         :type acl: :class:`gcloud.storage.acl.ACL`, or a compatible list.
         :param acl: The ACL object to save.  If left blank, this will save
                     current entries.
-
-        :rtype: :class:`gcloud.storage.acl.BucketACL`
-        :returns: The current ACL.
         """
         if acl is None:
             acl = self
@@ -454,8 +433,6 @@ def save(self, acl=None):
                 self.add_entity(self.entity_from_dict(entry))
             self.loaded = True
 
-        return self
-
     def clear(self):
         """Remove all ACL entries.
 
@@ -477,11 +454,8 @@ def clear(self):
           >>> acl.clear()
 
         At this point all the custom rules you created have been removed.
-
-        :rtype: :class:`gcloud.storage.acl.BucketACL`
-        :returns: The current ACL.
         """
-        return self.save([])
+        self.save([])
 
 
 class DefaultObjectACL(BucketACL):
@@ -502,11 +476,7 @@ def __init__(self, blob):
         self.blob = blob
 
     def reload(self):
-        """Reload the ACL data from Cloud Storage.
-
-        :rtype: :class:`ObjectACL`
-        :returns: The current ACL.
-        """
+        """Reload the ACL data from Cloud Storage."""
         self.entities.clear()
 
         url_path = '%s/acl' % self.blob.path
@@ -515,8 +485,6 @@ def reload(self):
         for entry in found.get('items', ()):
             self.add_entity(self.entity_from_dict(entry))
 
-        return self
-
     def save(self, acl=None):
         """Save the ACL data for this blob.
 
@@ -539,8 +507,6 @@ def save(self, acl=None):
                 self.add_entity(self.entity_from_dict(entry))
             self.loaded = True
 
-        return self
-
     def clear(self):
         """Remove all ACL rules from the blob.
 
@@ -549,4 +515,4 @@ def clear(self):
         have access to a blob that you created even after you clear ACL
         rules with this method.
         """
-        return self.save([])
+        self.save([])

gcloud/storage/blob.py

@@ -419,13 +419,9 @@ def upload_from_string(self, data, content_type='text/plain'):
                               content_type=content_type)
 
     def make_public(self):
-        """Make this blob public giving all users read access.
-
-        :returns: The current object.
-        """
+        """Make this blob public giving all users read access."""
         self.acl.all().grant_read()
         self.acl.save()
-        return self
 
     cache_control = _scalar_property('cacheControl')
     """HTTP 'Cache-Control' header for this object.
@@ -639,7 +635,7 @@ def updated(self):
 
 
 class _UploadConfig(object):
-    """ Faux message FBO apitools' 'ConfigureRequest'.
+    """Faux message FBO apitools' 'ConfigureRequest'.
 
     Values extracted from apitools
     'samples/storage_sample/storage/storage_v1_client.py'

gcloud/storage/test__helpers.py

@@ -60,7 +60,7 @@ def test_reload(self):
     def test__patch_properties(self):
         connection = _Connection({'foo': 'Foo'})
         derived = self._derivedClass(connection, '/path')()
-        self.assertTrue(derived._patch_properties({'foo': 'Foo'}) is derived)
+        derived._patch_properties({'foo': 'Foo'})
         derived.patch()
         kw = connection._requested
         self.assertEqual(len(kw), 1)