Skip to content

Commit 04ada73

Browse files
committed
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.
1 parent 65341ae commit 04ada73

File tree

10 files changed

+45
-98
lines changed

10 files changed

+45
-98
lines changed

gcloud/connection.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
""" Shared implementation of connections to API servers."""
15+
"""Shared implementation of connections to API servers."""
1616

1717
import json
1818
from pkg_resources import get_distribution

gcloud/datastore/batch.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class Batch(object):
5757
"""
5858

5959
def __init__(self, dataset_id=None, connection=None):
60-
""" Construct a batch.
60+
"""Construct a batch.
6161
6262
:type dataset_id: :class:`str`.
6363
:param dataset_id: The ID of the dataset.

gcloud/pubsub/api.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
""" Define API functions (not bound to classes)."""
15+
"""Define API functions (not bound to classes)."""
1616

1717
from gcloud._helpers import get_default_project
1818
from gcloud.pubsub._implicit_environ import get_default_connection

gcloud/pubsub/subscription.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
""" Define API Subscriptions."""
15+
"""Define API Subscriptions."""
1616

1717
from gcloud.exceptions import NotFound
1818

gcloud/pubsub/topic.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
""" Define API Topics."""
15+
"""Define API Topics."""
1616

1717
import base64
1818

gcloud/storage/_helpers.py

+2-15
Original file line numberDiff line numberDiff line change
@@ -74,17 +74,12 @@ def batch(self):
7474
return _PropertyBatch(self)
7575

7676
def reload(self):
77-
"""Reload properties from Cloud Storage.
78-
79-
:rtype: :class:`_PropertyMixin`
80-
:returns: The object you just reloaded data for.
81-
"""
77+
"""Reload properties from Cloud Storage."""
8278
# Pass only '?projection=noAcl' here because 'acl' and related
8379
# are handled via custom endpoints.
8480
query_params = {'projection': 'noAcl'}
8581
self._properties = self.connection.api_request(
8682
method='GET', path=self.path, query_params=query_params)
87-
return self
8883

8984
def _patch_properties(self, properties):
9085
"""Update particular fields of this object's properties.
@@ -97,21 +92,14 @@ def _patch_properties(self, properties):
9792
9893
:type properties: dict
9994
:param properties: The dictionary of values to update.
100-
101-
:rtype: :class:`_PropertyMixin`
102-
:returns: The current object.
10395
"""
10496
self._changes.update(properties.keys())
10597
self._properties.update(properties)
106-
return self
10798

10899
def patch(self):
109100
"""Sends all changed properties in a PATCH request.
110101
111-
Updates the ``properties`` with the response from the backend.
112-
113-
:rtype: :class:`Bucket`
114-
:returns: The current bucket.
102+
Updates the ``_properties`` with the response from the backend.
115103
"""
116104
# Pass '?projection=full' here because 'PATCH' documented not
117105
# to work properly w/ 'noAcl'.
@@ -120,7 +108,6 @@ def patch(self):
120108
self._properties = self.connection.api_request(
121109
method='PATCH', path=self.path, data=update_properties,
122110
query_params={'projection': 'full'})
123-
return self
124111

125112

126113
class _PropertyBatch(object):

gcloud/storage/acl.py

+15-49
Original file line numberDiff line numberDiff line change
@@ -127,55 +127,41 @@ def grant(self, role):
127127
128128
:type role: string
129129
:param role: The role to add to the entity.
130-
131-
:rtype: :class:`_ACLEntity`
132-
:returns: The entity class.
133130
"""
134131
self.roles.add(role)
135-
return self
136132

137133
def revoke(self, role):
138134
"""Remove a role from the entity.
139135
140136
:type role: string
141137
:param role: The role to remove from the entity.
142-
143-
:rtype: :class:`_ACLEntity`
144-
:returns: The entity class.
145138
"""
146139
if role in self.roles:
147140
self.roles.remove(role)
148-
return self
149141

150142
def grant_read(self):
151143
"""Grant read access to the current entity."""
152-
153-
return self.grant(_ACLEntity.READER_ROLE)
144+
self.grant(_ACLEntity.READER_ROLE)
154145

155146
def grant_write(self):
156147
"""Grant write access to the current entity."""
157-
158-
return self.grant(_ACLEntity.WRITER_ROLE)
148+
self.grant(_ACLEntity.WRITER_ROLE)
159149

160150
def grant_owner(self):
161151
"""Grant owner access to the current entity."""
162-
163-
return self.grant(_ACLEntity.OWNER_ROLE)
152+
self.grant(_ACLEntity.OWNER_ROLE)
164153

165154
def revoke_read(self):
166155
"""Revoke read access from the current entity."""
167-
168-
return self.revoke(_ACLEntity.READER_ROLE)
156+
self.revoke(_ACLEntity.READER_ROLE)
169157

170158
def revoke_write(self):
171159
"""Revoke write access from the current entity."""
172-
173-
return self.revoke(_ACLEntity.WRITER_ROLE)
160+
self.revoke(_ACLEntity.WRITER_ROLE)
174161

175162
def revoke_owner(self):
176163
"""Revoke owner access from the current entity."""
177-
178-
return self.revoke(_ACLEntity.OWNER_ROLE)
164+
self.revoke(_ACLEntity.OWNER_ROLE)
179165

180166

181167
class ACL(object):
@@ -234,7 +220,8 @@ def entity_from_dict(self, entity_dict):
234220
if not isinstance(entity, _ACLEntity):
235221
raise ValueError('Invalid dictionary: %s' % entity_dict)
236222

237-
return entity.grant(role)
223+
entity.grant(role)
224+
return entity
238225

239226
def has_entity(self, entity):
240227
"""Returns whether or not this ACL has any entries for an entity.
@@ -361,8 +348,9 @@ def get_entities(self):
361348
def reload(self):
362349
"""Reload the ACL data from Cloud Storage.
363350
364-
:rtype: :class:`ACL`
365-
:returns: The current ACL.
351+
This is a virtual method, expected to be implemented by subclasses.
352+
353+
:raises: :class:`NotImplementedError`
366354
"""
367355
raise NotImplementedError
368356

@@ -396,11 +384,7 @@ def __init__(self, bucket):
396384
self.bucket = bucket
397385

398386
def reload(self):
399-
"""Reload the ACL data from Cloud Storage.
400-
401-
:rtype: :class:`gcloud.storage.acl.BucketACL`
402-
:returns: The current ACL.
403-
"""
387+
"""Reload the ACL data from Cloud Storage."""
404388
self.entities.clear()
405389

406390
url_path = '%s/%s' % (self.bucket.path, self._URL_PATH_ELEM)
@@ -409,8 +393,6 @@ def reload(self):
409393
for entry in found.get('items', ()):
410394
self.add_entity(self.entity_from_dict(entry))
411395

412-
return self
413-
414396
def save(self, acl=None):
415397
"""Save this ACL for the current bucket.
416398
@@ -434,9 +416,6 @@ def save(self, acl=None):
434416
:type acl: :class:`gcloud.storage.acl.ACL`, or a compatible list.
435417
:param acl: The ACL object to save. If left blank, this will save
436418
current entries.
437-
438-
:rtype: :class:`gcloud.storage.acl.BucketACL`
439-
:returns: The current ACL.
440419
"""
441420
if acl is None:
442421
acl = self
@@ -454,8 +433,6 @@ def save(self, acl=None):
454433
self.add_entity(self.entity_from_dict(entry))
455434
self.loaded = True
456435

457-
return self
458-
459436
def clear(self):
460437
"""Remove all ACL entries.
461438
@@ -477,11 +454,8 @@ def clear(self):
477454
>>> acl.clear()
478455
479456
At this point all the custom rules you created have been removed.
480-
481-
:rtype: :class:`gcloud.storage.acl.BucketACL`
482-
:returns: The current ACL.
483457
"""
484-
return self.save([])
458+
self.save([])
485459

486460

487461
class DefaultObjectACL(BucketACL):
@@ -502,11 +476,7 @@ def __init__(self, blob):
502476
self.blob = blob
503477

504478
def reload(self):
505-
"""Reload the ACL data from Cloud Storage.
506-
507-
:rtype: :class:`ObjectACL`
508-
:returns: The current ACL.
509-
"""
479+
"""Reload the ACL data from Cloud Storage."""
510480
self.entities.clear()
511481

512482
url_path = '%s/acl' % self.blob.path
@@ -515,8 +485,6 @@ def reload(self):
515485
for entry in found.get('items', ()):
516486
self.add_entity(self.entity_from_dict(entry))
517487

518-
return self
519-
520488
def save(self, acl=None):
521489
"""Save the ACL data for this blob.
522490
@@ -539,8 +507,6 @@ def save(self, acl=None):
539507
self.add_entity(self.entity_from_dict(entry))
540508
self.loaded = True
541509

542-
return self
543-
544510
def clear(self):
545511
"""Remove all ACL rules from the blob.
546512
@@ -549,4 +515,4 @@ def clear(self):
549515
have access to a blob that you created even after you clear ACL
550516
rules with this method.
551517
"""
552-
return self.save([])
518+
self.save([])

gcloud/storage/blob.py

+2-6
Original file line numberDiff line numberDiff line change
@@ -419,13 +419,9 @@ def upload_from_string(self, data, content_type='text/plain'):
419419
content_type=content_type)
420420

421421
def make_public(self):
422-
"""Make this blob public giving all users read access.
423-
424-
:returns: The current object.
425-
"""
422+
"""Make this blob public giving all users read access."""
426423
self.acl.all().grant_read()
427424
self.acl.save()
428-
return self
429425

430426
cache_control = _scalar_property('cacheControl')
431427
"""HTTP 'Cache-Control' header for this object.
@@ -639,7 +635,7 @@ def updated(self):
639635

640636

641637
class _UploadConfig(object):
642-
""" Faux message FBO apitools' 'ConfigureRequest'.
638+
"""Faux message FBO apitools' 'ConfigureRequest'.
643639
644640
Values extracted from apitools
645641
'samples/storage_sample/storage/storage_v1_client.py'

gcloud/storage/test__helpers.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ def test_reload(self):
7474
def test__patch_properties(self):
7575
connection = _Connection({'foo': 'Foo'})
7676
derived = self._derivedClass(connection, '/path')()
77-
self.assertTrue(derived._patch_properties({'foo': 'Foo'}) is derived)
77+
derived._patch_properties({'foo': 'Foo'})
7878
derived.patch()
7979
kw = connection._requested
8080
self.assertEqual(len(kw), 1)

0 commit comments

Comments
 (0)