From f1539969d1a5a9f175357c0c878cd6b49a942649 Mon Sep 17 00:00:00 2001 From: Cathy Ouyang Date: Wed, 10 Aug 2022 15:47:49 -0700 Subject: [PATCH 1/3] feat: enable delete_blobs() to preserve generation --- google/cloud/storage/bucket.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 5408b9373..3798bd5b2 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -1698,6 +1698,7 @@ def delete_blobs( blobs, on_error=None, client=None, + preserve_generation=False, timeout=_DEFAULT_TIMEOUT, if_generation_match=None, if_generation_not_match=None, @@ -1725,6 +1726,11 @@ def delete_blobs( :param client: (Optional) The client to use. If not passed, falls back to the ``client`` stored on the current bucket. + :type preserve_generation: bool + :param preserve_generation: (Optional) Preserves blob generation if set to True. A list of + :class:`~google.cloud.storage.blob.Blob`-s is required to preserve generation. + Default: False. + :type if_generation_match: list of long :param if_generation_match: (Optional) See :ref:`using-if-generation-match` @@ -1787,11 +1793,20 @@ def delete_blobs( for blob in blobs: try: blob_name = blob + generation = None if not isinstance(blob_name, str): blob_name = blob.name + generation = blob.generation if preserve_generation else None + else: + if preserve_generation: + raise ValueError( + "A list of blob instances need to be passed in to preserve blob generations." + ) + self.delete_blob( blob_name, client=client, + generation=generation, if_generation_match=next(if_generation_match, None), if_generation_not_match=next(if_generation_not_match, None), if_metageneration_match=next(if_metageneration_match, None), From 4eeccdcc5255dcb81cc409492523d8d8ab88d316 Mon Sep 17 00:00:00 2001 From: Cathy Ouyang Date: Thu, 11 Aug 2022 15:07:14 -0700 Subject: [PATCH 2/3] update tests and docstring --- google/cloud/storage/bucket.py | 3 ++ tests/unit/test_bucket.py | 63 ++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 3798bd5b2..50992b927 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -1710,6 +1710,9 @@ def delete_blobs( Uses :meth:`delete_blob` to delete each individual blob. + By default, the latest versions of the blobs are deleted. Set `preserve_generation` + to True if blob generation should be propagated from the list of blobs. + If :attr:`user_project` is set, bills the API request to that project. :type blobs: list diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index d5206f287..17ea19018 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -1490,6 +1490,7 @@ def test_delete_w_force_w_user_project_w_miss_on_blob(self): bucket.delete_blob.assert_called_once_with( blob_name, client=client, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1649,6 +1650,7 @@ def test_delete_blobs_hit_w_explicit_client_w_timeout(self): bucket.delete_blob.assert_called_once_with( blob_name, client=client, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1693,6 +1695,7 @@ def test_delete_blobs_w_generation_match_w_retry(self): call_1 = mock.call( blob_name, client=None, + generation=None, if_generation_match=generation_number, if_generation_not_match=None, if_metageneration_match=None, @@ -1703,6 +1706,7 @@ def test_delete_blobs_w_generation_match_w_retry(self): call_2 = mock.call( blob_name2, client=None, + generation=None, if_generation_match=generation_number2, if_generation_not_match=None, if_metageneration_match=None, @@ -1730,6 +1734,7 @@ def test_delete_blobs_w_generation_match_none(self): call_1 = mock.call( blob_name, client=None, + generation=None, if_generation_match=generation_number, if_generation_not_match=None, if_metageneration_match=None, @@ -1740,6 +1745,7 @@ def test_delete_blobs_w_generation_match_none(self): call_2 = mock.call( blob_name2, client=None, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1749,6 +1755,59 @@ def test_delete_blobs_w_generation_match_none(self): ) bucket.delete_blob.assert_has_calls([call_1, call_2]) + def test_delete_blobs_w_preserve_generation(self): + name = "name" + blob_name = "blob-name" + blob_name2 = "blob-name2" + generation_number = 1234567890 + generation_number2 = 7890123456 + client = mock.Mock(spec=[]) + bucket = self._make_one(client=client, name=name) + blob = self._make_blob(bucket.name, blob_name) + blob.generation = generation_number + blob2 = self._make_blob(bucket.name, blob_name2) + blob2.generation = generation_number2 + bucket.delete_blob = mock.Mock() + retry = mock.Mock(spec=[]) + + # Test raises ValueError when unable to preserve generation from list of blob names + with pytest.raises( + ValueError, + match="A list of blob instances need to be passed in to preserve blob generations.", + ): + bucket.delete_blobs([blob_name, blob_name2], preserve_generation=True) + + # Test generation is propagated from list of blob instances + bucket.delete_blobs( + [blob, blob2], + preserve_generation=True, + retry=retry, + ) + + call_1 = mock.call( + blob_name, + client=None, + generation=generation_number, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=retry, + ) + call_2 = mock.call( + blob_name2, + client=None, + generation=generation_number2, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + timeout=self._get_default_timeout(), + retry=retry, + ) + bucket.delete_blob.assert_has_calls([call_1, call_2]) + def test_delete_blobs_miss_wo_on_error(self): from google.cloud.exceptions import NotFound @@ -1766,6 +1825,7 @@ def test_delete_blobs_miss_wo_on_error(self): call_1 = mock.call( blob_name, client=None, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1776,6 +1836,7 @@ def test_delete_blobs_miss_wo_on_error(self): call_2 = mock.call( blob_name2, client=None, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1804,6 +1865,7 @@ def test_delete_blobs_miss_w_on_error(self): call_1 = mock.call( blob_name, client=None, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, @@ -1814,6 +1876,7 @@ def test_delete_blobs_miss_w_on_error(self): call_2 = mock.call( blob_name2, client=None, + generation=None, if_generation_match=None, if_generation_not_match=None, if_metageneration_match=None, From 6871f5b25b6ab9722cfae9bd0cc68880aacb8aa1 Mon Sep 17 00:00:00 2001 From: Cathy Ouyang Date: Mon, 15 Aug 2022 15:41:47 -0700 Subject: [PATCH 3/3] address comments and update wording --- google/cloud/storage/bucket.py | 15 ++++++--------- tests/unit/test_bucket.py | 7 ------- 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index faf7297ae..6f133b923 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -1710,8 +1710,9 @@ def delete_blobs( Uses :meth:`delete_blob` to delete each individual blob. - By default, the latest versions of the blobs are deleted. Set `preserve_generation` - to True if blob generation should be propagated from the list of blobs. + By default, any generation information in the list of blobs is ignored, and the + live versions of all blobs are deleted. Set `preserve_generation` to True + if blob generation should instead be propagated from the list of blobs. If :attr:`user_project` is set, bills the API request to that project. @@ -1730,8 +1731,9 @@ def delete_blobs( to the ``client`` stored on the current bucket. :type preserve_generation: bool - :param preserve_generation: (Optional) Preserves blob generation if set to True. A list of - :class:`~google.cloud.storage.blob.Blob`-s is required to preserve generation. + :param preserve_generation: (Optional) Deletes only the generation specified on the blob object, + instead of the live version, if set to True. Only :class:~google.cloud.storage.blob.Blob + objects can have their generation set in this way. Default: False. :type if_generation_match: list of long @@ -1800,11 +1802,6 @@ def delete_blobs( if not isinstance(blob_name, str): blob_name = blob.name generation = blob.generation if preserve_generation else None - else: - if preserve_generation: - raise ValueError( - "A list of blob instances need to be passed in to preserve blob generations." - ) self.delete_blob( blob_name, diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index 17ea19018..5ff758758 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -1770,13 +1770,6 @@ def test_delete_blobs_w_preserve_generation(self): bucket.delete_blob = mock.Mock() retry = mock.Mock(spec=[]) - # Test raises ValueError when unable to preserve generation from list of blob names - with pytest.raises( - ValueError, - match="A list of blob instances need to be passed in to preserve blob generations.", - ): - bucket.delete_blobs([blob_name, blob_name2], preserve_generation=True) - # Test generation is propagated from list of blob instances bucket.delete_blobs( [blob, blob2],