Skip to content

Commit

Permalink
Updating delete methods on Blob and Bucket to accept a client.
Browse files Browse the repository at this point in the history 
Towards #952, removing connection from methods / constructors.
  • Loading branch information
@dhermes
dhermes committed Jul 13, 2015
1 parent c30132b commit 3c9b7e3
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 46 deletions.
15 changes: 6 additions & 9 deletions gcloud/storage/blob.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -266,28 +266,25 @@ def rename(self, new_name, client=None):
:rtype: :class:`Blob`
:returns: The newly-copied blob.
"""
connection = self._client_or_connection(client)
new_blob = self.bucket.copy_blob(self, self.bucket, new_name,
client=client)
self.delete(connection=connection)
self.delete(client=client)
return new_blob

def delete(self, connection=None):
def delete(self, client=None):
"""Deletes a blob from Cloud Storage.
:type connection: :class:`gcloud.storage.connection.Connection` or
``NoneType``
:param connection: Optional. The connection to use when sending
requests. If not provided, falls back to default.
:type client: :class:`gcloud.storage.client.Client` or ``NoneType``
:param client: Optional. The client to use. If not passed, falls back
to default connection.
:rtype: :class:`Blob`
:returns: The blob that was just deleted.
:raises: :class:`gcloud.exceptions.NotFound`
(propagated from
:meth:`gcloud.storage.bucket.Bucket.delete_blob`).
"""
connection = _require_connection(connection)
return self.bucket.delete_blob(self.name, connection=connection)
return self.bucket.delete_blob(self.name, client=client)

def download_to_file(self, file_obj, client=None):
"""Download the contents of this blob into a file-like object.
Expand Down
36 changes: 16 additions & 20 deletions gcloud/storage/bucket.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ def list_blobs(self, max_results=None, page_token=None, prefix=None,
result.next_page_token = page_token
return result

def delete(self, force=False, connection=None):
def delete(self, force=False, client=None):
"""Delete this bucket.
The bucket **must** be empty in order to submit a delete request. If
Expand All @@ -330,15 +330,14 @@ def delete(self, force=False, connection=None):
:type force: boolean
:param force: If True, empties the bucket's objects then deletes it.
:type connection: :class:`gcloud.storage.connection.Connection` or
``NoneType``
:param connection: Optional. The connection to use when sending
requests. If not provided, falls back to default.
:type client: :class:`gcloud.storage.client.Client` or ``NoneType``
:param client: Optional. The client to use. If not passed, falls back
to default connection.
:raises: :class:`ValueError` if ``force`` is ``True`` and the bucket
contains more than 256 objects / blobs.
"""
connection = _require_connection(connection)
connection = self._client_or_connection(client)
if force:
blobs = list(self.list_blobs(
max_results=self._MAX_OBJECTS_FOR_ITERATION + 1,
Expand All @@ -354,15 +353,15 @@ def delete(self, force=False, connection=None):

# Ignore 404 errors on delete.
self.delete_blobs(blobs, on_error=lambda blob: None,
connection=connection)
client=client)

# We intentionally pass `_target_object=None` since a DELETE
# request has no response value (whether in a standard request or
# in a batch request).
connection.api_request(method='DELETE', path=self.path,
_target_object=None)

def delete_blob(self, blob_name, connection=None):
def delete_blob(self, blob_name, client=None):
"""Deletes a blob from the current bucket.
If the blob isn't found (backend 404), raises a
Expand All @@ -385,26 +384,25 @@ def delete_blob(self, blob_name, connection=None):
:type blob_name: string
:param blob_name: A blob name to delete.
:type connection: :class:`gcloud.storage.connection.Connection` or
``NoneType``
:param connection: Optional. The connection to use when sending
requests. If not provided, falls back to default.
:type client: :class:`gcloud.storage.client.Client` or ``NoneType``
:param client: Optional. The client to use. If not passed, falls back
to default connection.
:raises: :class:`gcloud.exceptions.NotFound` (to suppress
the exception, call ``delete_blobs``, passing a no-op
``on_error`` callback, e.g.::
>>> bucket.delete_blobs([blob], on_error=lambda blob: None)
"""
connection = _require_connection(connection)
connection = self._client_or_connection(client)
blob_path = Blob.path_helper(self.path, blob_name)
# We intentionally pass `_target_object=None` since a DELETE
# request has no response value (whether in a standard request or
# in a batch request).
connection.api_request(method='DELETE', path=blob_path,
_target_object=None)

def delete_blobs(self, blobs, on_error=None, connection=None):
def delete_blobs(self, blobs, on_error=None, client=None):
"""Deletes a list of blobs from the current bucket.
Uses :func:`Bucket.delete_blob` to delete each individual blob.
Expand All @@ -417,21 +415,19 @@ def delete_blobs(self, blobs, on_error=None, connection=None):
:class:`gcloud.exceptions.NotFound`;
otherwise, the exception is propagated.
:type connection: :class:`gcloud.storage.connection.Connection` or
``NoneType``
:param connection: Optional. The connection to use when sending
requests. If not provided, falls back to default.
:type client: :class:`gcloud.storage.client.Client` or ``NoneType``
:param client: Optional. The client to use. If not passed, falls back
to default connection.
:raises: :class:`gcloud.exceptions.NotFound` (if
`on_error` is not passed).
"""
connection = _require_connection(connection)
for blob in blobs:
try:
blob_name = blob
if not isinstance(blob_name, six.string_types):
blob_name = blob.name
self.delete_blob(blob_name, connection=connection)
self.delete_blob(blob_name, client=client)
except NotFound:
if on_error is not None:
on_error(blob)
Expand Down
12 changes: 6 additions & 6 deletions gcloud/storage/test_blob.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ def test_rename(self):
self.assertEqual(blob.name, BLOB_NAME)
self.assertEqual(new_blob.name, NEW_NAME)
self.assertFalse(BLOB_NAME in bucket._blobs)
self.assertEqual(bucket._deleted, [(BLOB_NAME, connection)])
self.assertEqual(bucket._deleted, [(BLOB_NAME, client)])
self.assertTrue(NEW_NAME in bucket._blobs)

def test_delete_w_implicit_connection(self):
Expand All @@ -312,7 +312,7 @@ def test_delete_w_implicit_connection(self):
with _monkey_defaults(connection=connection):
blob.delete()
self.assertFalse(blob.exists(client=client))
self.assertEqual(bucket._deleted, [(BLOB_NAME, connection)])
self.assertEqual(bucket._deleted, [(BLOB_NAME, None)])

def test_delete_w_explicit_connection(self):
from six.moves.http_client import NOT_FOUND
Expand All @@ -323,9 +323,9 @@ def test_delete_w_explicit_connection(self):
bucket = _Bucket()
blob = self._makeOne(BLOB_NAME, bucket=bucket)
bucket._blobs[BLOB_NAME] = 1
blob.delete(connection=connection)
blob.delete(client=client)
self.assertFalse(blob.exists(client=client))
self.assertEqual(bucket._deleted, [(BLOB_NAME, connection)])
self.assertEqual(bucket._deleted, [(BLOB_NAME, client)])

def _download_to_file_helper(self, chunk_size=None):
from six.moves.http_client import OK
Expand Down Expand Up @@ -1147,9 +1147,9 @@ def copy_blob(self, blob, destination_bucket, new_name, client=None):
destination_bucket._blobs[new_name] = self._blobs[blob.name]
return blob.__class__(new_name, bucket=destination_bucket)

def delete_blob(self, blob_name, connection=None):
def delete_blob(self, blob_name, client=None):
del self._blobs[blob_name]
self._deleted.append((blob_name, connection))
self._deleted.append((blob_name, client))


class _Signer(object):
Expand Down
33 changes: 22 additions & 11 deletions gcloud/storage/test_bucket.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,9 @@ def test_delete_default_miss(self):
from gcloud.exceptions import NotFound
NAME = 'name'
connection = _Connection()
client = _Client(connection)
bucket = self._makeOne(NAME)
self.assertRaises(NotFound, bucket.delete, connection=connection)
self.assertRaises(NotFound, bucket.delete, client=client)
expected_cw = [{
'method': 'DELETE',
'path': bucket.path,
Expand All @@ -361,8 +362,9 @@ def test_delete_explicit_hit(self):
GET_BLOBS_RESP = {'items': []}
connection = _Connection(GET_BLOBS_RESP)
connection._delete_bucket = True
client = _Client(connection)
bucket = self._makeOne(NAME)
result = bucket.delete(force=True, connection=connection)
result = bucket.delete(force=True, client=client)
self.assertTrue(result is None)
expected_cw = [{
'method': 'DELETE',
Expand All @@ -385,8 +387,9 @@ def test_delete_explicit_force_delete_blobs(self):
connection = _Connection(GET_BLOBS_RESP, DELETE_BLOB1_RESP,
DELETE_BLOB2_RESP)
connection._delete_bucket = True
client = _Client(connection)
bucket = self._makeOne(NAME)
result = bucket.delete(force=True, connection=connection)
result = bucket.delete(force=True, client=client)
self.assertTrue(result is None)
expected_cw = [{
'method': 'DELETE',
Expand All @@ -402,8 +405,9 @@ def test_delete_explicit_force_miss_blobs(self):
# Note the connection does not have a response for the blob.
connection = _Connection(GET_BLOBS_RESP)
connection._delete_bucket = True
client = _Client(connection)
bucket = self._makeOne(NAME)
result = bucket.delete(force=True, connection=connection)
result = bucket.delete(force=True, client=client)
self.assertTrue(result is None)
expected_cw = [{
'method': 'DELETE',
Expand All @@ -424,22 +428,24 @@ def test_delete_explicit_too_many(self):
}
connection = _Connection(GET_BLOBS_RESP)
connection._delete_bucket = True
client = _Client(connection)
bucket = self._makeOne(NAME)

# Make the Bucket refuse to delete with 2 objects.
bucket._MAX_OBJECTS_FOR_ITERATION = 1
self.assertRaises(ValueError, bucket.delete, force=True,
connection=connection)
client=client)
self.assertEqual(connection._deleted_buckets, [])

def test_delete_blob_miss(self):
from gcloud.exceptions import NotFound
NAME = 'name'
NONESUCH = 'nonesuch'
connection = _Connection()
client = _Client(connection)
bucket = self._makeOne(NAME)
self.assertRaises(NotFound, bucket.delete_blob, NONESUCH,
connection=connection)
client=client)
kw, = connection._requested
self.assertEqual(kw['method'], 'DELETE')
self.assertEqual(kw['path'], '/b/%s/o/%s' % (NAME, NONESUCH))
Expand All @@ -448,8 +454,9 @@ def test_delete_blob_hit(self):
NAME = 'name'
BLOB_NAME = 'blob-name'
connection = _Connection({})
client = _Client(connection)
bucket = self._makeOne(NAME)
result = bucket.delete_blob(BLOB_NAME, connection=connection)
result = bucket.delete_blob(BLOB_NAME, client=client)
self.assertTrue(result is None)
kw, = connection._requested
self.assertEqual(kw['method'], 'DELETE')
Expand All @@ -458,16 +465,18 @@ def test_delete_blob_hit(self):
def test_delete_blobs_empty(self):
NAME = 'name'
connection = _Connection()
client = _Client(connection)
bucket = self._makeOne(NAME)
bucket.delete_blobs([], connection=connection)
bucket.delete_blobs([], client=client)
self.assertEqual(connection._requested, [])

def test_delete_blobs_hit(self):
NAME = 'name'
BLOB_NAME = 'blob-name'
connection = _Connection({})
client = _Client(connection)
bucket = self._makeOne(NAME)
bucket.delete_blobs([BLOB_NAME], connection=connection)
bucket.delete_blobs([BLOB_NAME], client=client)
kw = connection._requested
self.assertEqual(len(kw), 1)
self.assertEqual(kw[0]['method'], 'DELETE')
Expand All @@ -479,9 +488,10 @@ def test_delete_blobs_miss_no_on_error(self):
BLOB_NAME = 'blob-name'
NONESUCH = 'nonesuch'
connection = _Connection({})
client = _Client(connection)
bucket = self._makeOne(NAME)
self.assertRaises(NotFound, bucket.delete_blobs, [BLOB_NAME, NONESUCH],
connection=connection)
client=client)
kw = connection._requested
self.assertEqual(len(kw), 2)
self.assertEqual(kw[0]['method'], 'DELETE')
Expand All @@ -494,10 +504,11 @@ def test_delete_blobs_miss_w_on_error(self):
BLOB_NAME = 'blob-name'
NONESUCH = 'nonesuch'
connection = _Connection({})
client = _Client(connection)
bucket = self._makeOne(NAME)
errors = []
bucket.delete_blobs([BLOB_NAME, NONESUCH], errors.append,
connection=connection)
client=client)
self.assertEqual(errors, [NONESUCH])
kw = connection._requested
self.assertEqual(len(kw), 2)
Expand Down

1 comment on commit 3c9b7e3

@tseaver
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit LGTM

Please sign in to comment.