From e0024ee9d5fa2e7446a2f77b26a5bf6fbc1a25db Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Mon, 21 Oct 2019 15:56:25 +0300 Subject: [PATCH 1/5] feat(storage): mark preserve_acl as deprecated --- storage/google/cloud/storage/bucket.py | 14 +++++++++---- storage/tests/unit/test_bucket.py | 29 +++++++------------------- 2 files changed, 17 insertions(+), 26 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 48ab09e23e4f..76aa9a876276 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -1064,7 +1064,7 @@ def copy_blob( destination_bucket, new_name=None, client=None, - preserve_acl=True, + preserve_acl=None, source_generation=None, ): """Copy the given blob to the given bucket, optionally with a new name. @@ -1088,7 +1088,9 @@ def copy_blob( :type preserve_acl: bool :param preserve_acl: Optional. Copies ACL from old blob to new blob. - Default: True. + Default: True. Deprecated: this argument is no longer + affecting anything and will be removed in a future release. + Please update the bucket's ACL manually. :type source_generation: long :param source_generation: Optional. The generation of the blob to be @@ -1118,8 +1120,12 @@ def copy_blob( _target_object=new_blob, ) - if not preserve_acl: - new_blob.acl.save(acl={}, client=client) + if preserve_acl is not None: + warnings.warn( + "The 'preserve_acl' argument is deprecated.", + DeprecationWarning, + stacklevel=2, + ) new_blob._set_properties(copy_result) return new_blob diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index de943339e200..54447e550220 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -1152,8 +1152,8 @@ def test_copy_blobs_source_generation(self): self.assertEqual(kw["path"], COPY_PATH) self.assertEqual(kw["query_params"], {"sourceGeneration": GENERATION}) - def test_copy_blobs_preserve_acl(self): - from google.cloud.storage.acl import ObjectACL + @mock.patch("warnings.warn") + def test_copy_blobs_preserve_acl_deprecated(self, mock_warn): SOURCE = "source" DEST = "dest" @@ -1166,27 +1166,12 @@ def test_copy_blobs_preserve_acl(self): dest = self._make_one(client=client, name=DEST) blob = self._make_blob(SOURCE, BLOB_NAME) - new_blob = source.copy_blob( - blob, dest, NEW_NAME, client=client, preserve_acl=False - ) - - self.assertIs(new_blob.bucket, dest) - self.assertEqual(new_blob.name, NEW_NAME) - self.assertIsInstance(new_blob.acl, ObjectACL) - - kw1, kw2 = connection._requested - COPY_PATH = "/b/{}/o/{}/copyTo/b/{}/o/{}".format( - SOURCE, BLOB_NAME, DEST, NEW_NAME + source.copy_blob(blob, dest, NEW_NAME, client=client, preserve_acl=False) + mock_warn.assert_called_once_with( + "The 'preserve_acl' argument is deprecated.", + DeprecationWarning, + stacklevel=2, ) - NEW_BLOB_PATH = "/b/{}/o/{}".format(DEST, NEW_NAME) - - self.assertEqual(kw1["method"], "POST") - self.assertEqual(kw1["path"], COPY_PATH) - self.assertEqual(kw1["query_params"], {}) - - self.assertEqual(kw2["method"], "PATCH") - self.assertEqual(kw2["path"], NEW_BLOB_PATH) - self.assertEqual(kw2["query_params"], {"projection": "full"}) def test_copy_blobs_w_name_and_user_project(self): SOURCE = "source" From adae5b013304d0e26e30d47ed2f1188d7d6b1a02 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Mon, 21 Oct 2019 16:43:41 +0300 Subject: [PATCH 2/5] update comments --- storage/google/cloud/storage/bucket.py | 4 ++-- storage/tests/unit/test_bucket.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 76aa9a876276..6a46abcf5c3b 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -1088,7 +1088,7 @@ def copy_blob( :type preserve_acl: bool :param preserve_acl: Optional. Copies ACL from old blob to new blob. - Default: True. Deprecated: this argument is no longer + Default: None. Deprecated: this argument is no longer affecting anything and will be removed in a future release. Please update the bucket's ACL manually. @@ -1122,7 +1122,7 @@ def copy_blob( if preserve_acl is not None: warnings.warn( - "The 'preserve_acl' argument is deprecated.", + "The 'preserve_acl' argument is deprecated. Please update the bucket's ACL manually.", DeprecationWarning, stacklevel=2, ) diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 54447e550220..dcd0b969fbc8 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -1168,7 +1168,7 @@ def test_copy_blobs_preserve_acl_deprecated(self, mock_warn): source.copy_blob(blob, dest, NEW_NAME, client=client, preserve_acl=False) mock_warn.assert_called_once_with( - "The 'preserve_acl' argument is deprecated.", + "The 'preserve_acl' argument is deprecated. Please update the bucket's ACL manually.", DeprecationWarning, stacklevel=2, ) From 54edcbdc15e8384d0364e892b15399c065fd6f20 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Fri, 25 Oct 2019 10:38:10 +0300 Subject: [PATCH 3/5] update deprecation info --- storage/google/cloud/storage/bucket.py | 5 ++++- storage/tests/unit/test_bucket.py | 27 +++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 6a46abcf5c3b..4aeb2d8e86a5 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -1122,10 +1122,13 @@ def copy_blob( if preserve_acl is not None: warnings.warn( - "The 'preserve_acl' argument is deprecated. Please update the bucket's ACL manually.", + "The 'preserve_acl' argument is deprecated. For forward compatibility, " + "please update the blob's ACL manually.", DeprecationWarning, stacklevel=2, ) + if not preserve_acl and preserve_acl is not None: + new_blob.acl.save(acl={}, client=client) new_blob._set_properties(copy_result) return new_blob diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index dcd0b969fbc8..d756a87dea96 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -1154,6 +1154,7 @@ def test_copy_blobs_source_generation(self): @mock.patch("warnings.warn") def test_copy_blobs_preserve_acl_deprecated(self, mock_warn): + from google.cloud.storage.acl import ObjectACL SOURCE = "source" DEST = "dest" @@ -1166,13 +1167,33 @@ def test_copy_blobs_preserve_acl_deprecated(self, mock_warn): dest = self._make_one(client=client, name=DEST) blob = self._make_blob(SOURCE, BLOB_NAME) - source.copy_blob(blob, dest, NEW_NAME, client=client, preserve_acl=False) - mock_warn.assert_called_once_with( - "The 'preserve_acl' argument is deprecated. Please update the bucket's ACL manually.", + new_blob = source.copy_blob(blob, dest, NEW_NAME, client=client, preserve_acl=False) + + mock_warn.assert_called_with( + "The 'preserve_acl' argument is deprecated. For forward compatibility, " + "please update the blob's ACL manually.", DeprecationWarning, stacklevel=2, ) + self.assertIs(new_blob.bucket, dest) + self.assertEqual(new_blob.name, NEW_NAME) + self.assertIsInstance(new_blob.acl, ObjectACL) + + kw1, kw2 = connection._requested + COPY_PATH = "/b/{}/o/{}/copyTo/b/{}/o/{}".format( + SOURCE, BLOB_NAME, DEST, NEW_NAME + ) + NEW_BLOB_PATH = "/b/{}/o/{}".format(DEST, NEW_NAME) + + self.assertEqual(kw1["method"], "POST") + self.assertEqual(kw1["path"], COPY_PATH) + self.assertEqual(kw1["query_params"], {}) + + self.assertEqual(kw2["method"], "PATCH") + self.assertEqual(kw2["path"], NEW_BLOB_PATH) + self.assertEqual(kw2["query_params"], {"projection": "full"}) + def test_copy_blobs_w_name_and_user_project(self): SOURCE = "source" DEST = "dest" From 2ec2e1f98c0cd995106cd259f803f7e49070258c Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Fri, 25 Oct 2019 11:03:18 +0300 Subject: [PATCH 4/5] black --- storage/tests/unit/test_bucket.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index d756a87dea96..52e9d38a1d80 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -1167,7 +1167,9 @@ def test_copy_blobs_preserve_acl_deprecated(self, mock_warn): dest = self._make_one(client=client, name=DEST) blob = self._make_blob(SOURCE, BLOB_NAME) - new_blob = source.copy_blob(blob, dest, NEW_NAME, client=client, preserve_acl=False) + new_blob = source.copy_blob( + blob, dest, NEW_NAME, client=client, preserve_acl=False + ) mock_warn.assert_called_with( "The 'preserve_acl' argument is deprecated. For forward compatibility, " From 3ff8d4eb1437a2bbf3915ef62b0b6569f48dbbd1 Mon Sep 17 00:00:00 2001 From: Leonid Emar-Kar Date: Thu, 31 Oct 2019 11:00:10 +0300 Subject: [PATCH 5/5] fix comment --- storage/google/cloud/storage/bucket.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 4aeb2d8e86a5..eabf883455ad 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -1090,7 +1090,7 @@ def copy_blob( :param preserve_acl: Optional. Copies ACL from old blob to new blob. Default: None. Deprecated: this argument is no longer affecting anything and will be removed in a future release. - Please update the bucket's ACL manually. + Please update the blob's ACL manually. :type source_generation: long :param source_generation: Optional. The generation of the blob to be