From 04ada738d10705a5960a28b7cbef99df8bf14bc5 Mon Sep 17 00:00:00 2001 From: Danny Hermes Date: Mon, 30 Mar 2015 19:38:44 -0700 Subject: [PATCH] Remove uses of 'return self' that don't return copies. Fixes #776. Remaining uses of `return self` are in tests, __enter__ in context managers and the _LazyProperty descriptor. --- gcloud/connection.py | 2 +- gcloud/datastore/batch.py | 2 +- gcloud/pubsub/api.py | 2 +- gcloud/pubsub/subscription.py | 2 +- gcloud/pubsub/topic.py | 2 +- gcloud/storage/_helpers.py | 17 ++------- gcloud/storage/acl.py | 64 ++++++++------------------------- gcloud/storage/blob.py | 8 ++--- gcloud/storage/test__helpers.py | 2 +- gcloud/storage/test_acl.py | 42 +++++++++++----------- 10 files changed, 45 insertions(+), 98 deletions(-) diff --git a/gcloud/connection.py b/gcloud/connection.py index 70c44d596acd..f70b1e2720f8 100644 --- a/gcloud/connection.py +++ b/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 diff --git a/gcloud/datastore/batch.py b/gcloud/datastore/batch.py index 2576268344d2..1d568f47f199 100644 --- a/gcloud/datastore/batch.py +++ b/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. diff --git a/gcloud/pubsub/api.py b/gcloud/pubsub/api.py index 56f826ffecd9..8fb2c9bcc57f 100644 --- a/gcloud/pubsub/api.py +++ b/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 diff --git a/gcloud/pubsub/subscription.py b/gcloud/pubsub/subscription.py index 7464a4e46879..4d438c3010fc 100644 --- a/gcloud/pubsub/subscription.py +++ b/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 diff --git a/gcloud/pubsub/topic.py b/gcloud/pubsub/topic.py index d9a7b9b16ecb..bbb0d7ed2b38 100644 --- a/gcloud/pubsub/topic.py +++ b/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 diff --git a/gcloud/storage/_helpers.py b/gcloud/storage/_helpers.py index c65f4ecf02bd..eb35ec62f604 100644 --- a/gcloud/storage/_helpers.py +++ b/gcloud/storage/_helpers.py @@ -74,17 +74,12 @@ def batch(self): return _PropertyBatch(self) 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. @@ -97,21 +92,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'. @@ -120,7 +108,6 @@ def patch(self): self._properties = self.connection.api_request( method='PATCH', path=self.path, data=update_properties, query_params={'projection': 'full'}) - return self class _PropertyBatch(object): diff --git a/gcloud/storage/acl.py b/gcloud/storage/acl.py index 48b971e6d8b7..a2eaf01b2f97 100644 --- a/gcloud/storage/acl.py +++ b/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([]) diff --git a/gcloud/storage/blob.py b/gcloud/storage/blob.py index 8bc66f235f7c..1856beeaa93a 100644 --- a/gcloud/storage/blob.py +++ b/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' diff --git a/gcloud/storage/test__helpers.py b/gcloud/storage/test__helpers.py index 52fab616b149..33011dfe37fa 100644 --- a/gcloud/storage/test__helpers.py +++ b/gcloud/storage/test__helpers.py @@ -74,7 +74,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) diff --git a/gcloud/storage/test_acl.py b/gcloud/storage/test_acl.py index 2991aa427168..c998cac895c5 100644 --- a/gcloud/storage/test_acl.py +++ b/gcloud/storage/test_acl.py @@ -54,8 +54,7 @@ def test_grant_simple(self): TYPE = 'type' ROLE = 'role' entity = self._makeOne(TYPE) - found = entity.grant(ROLE) - self.assertTrue(found is entity) + entity.grant(ROLE) self.assertEqual(entity.get_roles(), set([ROLE])) def test_grant_duplicate(self): @@ -72,8 +71,7 @@ def test_revoke_miss(self): TYPE = 'type' ROLE = 'nonesuch' entity = self._makeOne(TYPE) - found = entity.revoke(ROLE) - self.assertTrue(found is entity) + entity.revoke(ROLE) self.assertEqual(entity.get_roles(), set()) def test_revoke_hit(self): @@ -545,7 +543,7 @@ def test_reload_eager_missing(self): acl = self._makeOne(bucket) acl.loaded = True acl.entity('allUsers', ROLE) - self.assertTrue(acl.reload() is acl) + acl.reload() self.assertEqual(list(acl), []) kw = connection._requested self.assertEqual(len(kw), 1) @@ -560,7 +558,7 @@ def test_reload_eager_empty(self): acl = self._makeOne(bucket) acl.loaded = True acl.entity('allUsers', ROLE) - self.assertTrue(acl.reload() is acl) + acl.reload() self.assertEqual(list(acl), []) kw = connection._requested self.assertEqual(len(kw), 1) @@ -575,7 +573,7 @@ def test_reload_eager_nonempty(self): bucket = _Bucket(NAME, connection) acl = self._makeOne(bucket) acl.loaded = True - self.assertTrue(acl.reload() is acl) + acl.reload() self.assertEqual(list(acl), [{'entity': 'allUsers', 'role': ROLE}]) kw = connection._requested self.assertEqual(len(kw), 1) @@ -589,7 +587,7 @@ def test_reload_lazy(self): {'items': [{'entity': 'allUsers', 'role': ROLE}]}) bucket = _Bucket(NAME, connection) acl = self._makeOne(bucket) - self.assertTrue(acl.reload() is acl) + acl.reload() self.assertEqual(list(acl), [{'entity': 'allUsers', 'role': ROLE}]) kw = connection._requested self.assertEqual(len(kw), 1) @@ -601,7 +599,7 @@ def test_save_none_set_none_passed(self): connection = _Connection() bucket = _Bucket(NAME, connection) acl = self._makeOne(bucket) - self.assertTrue(acl.save() is acl) + acl.save() kw = connection._requested self.assertEqual(len(kw), 0) @@ -611,7 +609,7 @@ def test_save_existing_missing_none_passed(self): bucket = _Bucket(NAME, connection) acl = self._makeOne(bucket) acl.loaded = True - self.assertTrue(acl.save() is acl) + acl.save() self.assertEqual(list(acl), []) kw = connection._requested self.assertEqual(len(kw), 1) @@ -629,7 +627,7 @@ def test_save_no_arg(self): acl = self._makeOne(bucket) acl.loaded = True acl.entity('allUsers').grant(ROLE) - self.assertTrue(acl.save() is acl) + acl.save() self.assertEqual(list(acl), AFTER) kw = connection._requested self.assertEqual(len(kw), 1) @@ -648,7 +646,7 @@ def test_save_w_arg(self): bucket = _Bucket(NAME, connection) acl = self._makeOne(bucket) acl.loaded = True - self.assertTrue(acl.save(new_acl) is acl) + acl.save(new_acl) entries = list(acl) self.assertEqual(len(entries), 2) self.assertTrue(STICKY in entries) @@ -670,7 +668,7 @@ def test_clear(self): acl = self._makeOne(bucket) acl.loaded = True acl.entity('allUsers', ROLE1) - self.assertTrue(acl.clear() is acl) + acl.clear() self.assertEqual(list(acl), [STICKY]) kw = connection._requested self.assertEqual(len(kw), 1) @@ -708,7 +706,7 @@ def test_reload_eager_missing(self): acl = self._makeOne(blob) acl.loaded = True acl.entity('allUsers', ROLE) - self.assertTrue(acl.reload() is acl) + acl.reload() self.assertEqual(list(acl), []) def test_reload_eager_empty(self): @@ -722,7 +720,7 @@ def test_reload_eager_empty(self): acl = self._makeOne(blob) acl.loaded = True acl.entity('allUsers', ROLE) - self.assertTrue(acl.reload() is acl) + acl.reload() self.assertEqual(list(acl), []) def test_reload_eager_nonempty(self): @@ -735,7 +733,7 @@ def test_reload_eager_nonempty(self): blob = _Blob(bucket, BLOB_NAME) acl = self._makeOne(blob) acl.loaded = True - self.assertTrue(acl.reload() is acl) + acl.reload() self.assertEqual(list(acl), after['items']) kw = connection._requested self.assertEqual(len(kw), 1) @@ -751,7 +749,7 @@ def test_reload_lazy(self): bucket = _Bucket(NAME, connection) blob = _Blob(bucket, BLOB_NAME) acl = self._makeOne(blob) - self.assertTrue(acl.reload() is acl) + acl.reload() self.assertEqual(list(acl), [{'entity': 'allUsers', 'role': ROLE}]) kw = connection._requested @@ -766,7 +764,7 @@ def test_save_none_set_none_passed(self): bucket = _Bucket(NAME, connection) blob = _Blob(bucket, BLOB_NAME) acl = self._makeOne(blob) - self.assertTrue(acl.save() is acl) + acl.save() kw = connection._requested self.assertEqual(len(kw), 0) @@ -779,7 +777,7 @@ def test_save_existing_missing_none_passed(self): blob = _Blob(bucket, BLOB_NAME) acl = self._makeOne(blob) acl.loaded = True - self.assertTrue(acl.save() is acl) + acl.save() kw = connection._requested self.assertEqual(len(kw), 1) self.assertEqual(kw[0]['method'], 'PATCH') @@ -795,7 +793,7 @@ def test_save_existing_set_none_passed(self): blob = _Blob(bucket, BLOB_NAME) acl = self._makeOne(blob) acl.loaded = True - self.assertTrue(acl.save() is acl) + acl.save() kw = connection._requested self.assertEqual(len(kw), 1) self.assertEqual(kw[0]['method'], 'PATCH') @@ -814,7 +812,7 @@ def test_save_existing_set_new_passed(self): acl = self._makeOne(blob) acl.loaded = True acl.entity('allUsers', 'other-role') - self.assertTrue(acl.save(new_acl) is acl) + acl.save(new_acl) self.assertEqual(list(acl), new_acl) kw = connection._requested self.assertEqual(len(kw), 1) @@ -833,7 +831,7 @@ def test_clear(self): acl = self._makeOne(blob) acl.loaded = True acl.entity('allUsers', ROLE) - self.assertTrue(acl.clear() is acl) + acl.clear() self.assertEqual(list(acl), []) kw = connection._requested self.assertEqual(len(kw), 1)