From edd01374ec95237ef4e5681f65bba1698a6d7c39 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 13 Apr 2020 12:32:21 +0300 Subject: [PATCH 1/3] refactor(storage): deprecate preserve_acl arg in copy_blob() method --- google/cloud/storage/bucket.py | 15 ++++++++++----- tests/unit/test_bucket.py | 30 ++++++++++++++++-------------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index c2a909357..6e15ca7f8 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -1244,7 +1244,7 @@ def copy_blob( destination_bucket, new_name=None, client=None, - preserve_acl=True, + preserve_acl=None, source_generation=None, timeout=_DEFAULT_TIMEOUT, ): @@ -1268,8 +1268,8 @@ def copy_blob( to the ``client`` stored on the current bucket. :type preserve_acl: bool - :param preserve_acl: (Optional) Copies ACL from old blob to new blob. - Default: True. + :param preserve_acl: DEPRECATED. (Optional) Copies ACL from old blob to + new blob. Default: True. :type source_generation: long :param source_generation: (Optional) The generation of the blob to be @@ -1307,8 +1307,13 @@ def copy_blob( timeout=timeout, ) - if not preserve_acl: - new_blob.acl.save(acl={}, client=client, timeout=timeout) + if preserve_acl is not None: + warnings.warn( + "preserve_acl arg is deprecated and will be removed in future." + "Do a subsequent ACL update instead.", + PendingDeprecationWarning, + stacklevel=1, + ) new_blob._set_properties(copy_result) return new_blob diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index 365e1f0e1..01eb8ec28 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -1144,29 +1144,31 @@ 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 - ) + with mock.patch("warnings.warn") as warn: + new_blob = source.copy_blob( + blob, dest, NEW_NAME, client=client, preserve_acl=False + ) + warn.assert_called_once_with( + "preserve_acl arg is deprecated and will be removed in future." + "Do a subsequent ACL update instead.", + PendingDeprecationWarning, + stacklevel=1, + ) 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(kw1["timeout"], self._get_default_timeout()) - - self.assertEqual(kw2["method"], "PATCH") - self.assertEqual(kw2["path"], NEW_BLOB_PATH) - self.assertEqual(kw2["query_params"], {"projection": "full"}) - self.assertEqual(kw2["timeout"], self._get_default_timeout()) + self.assertEqual(connection._requested[0]["method"], "POST") + self.assertEqual(connection._requested[0]["path"], COPY_PATH) + self.assertEqual(connection._requested[0]["query_params"], {}) + self.assertEqual( + connection._requested[0]["timeout"], self._get_default_timeout() + ) def test_copy_blobs_w_name_and_user_project(self): SOURCE = "source" From 088783872461a8b719b11c6c853bc2a4c688fc0d Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 17 Apr 2020 12:15:59 +0300 Subject: [PATCH 2/3] Revert "refactor(storage): deprecate preserve_acl arg in copy_blob() method" This reverts commit edd01374ec95237ef4e5681f65bba1698a6d7c39. --- google/cloud/storage/bucket.py | 15 +++++---------- tests/unit/test_bucket.py | 30 ++++++++++++++---------------- 2 files changed, 19 insertions(+), 26 deletions(-) diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index 6e15ca7f8..c2a909357 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -1244,7 +1244,7 @@ def copy_blob( destination_bucket, new_name=None, client=None, - preserve_acl=None, + preserve_acl=True, source_generation=None, timeout=_DEFAULT_TIMEOUT, ): @@ -1268,8 +1268,8 @@ def copy_blob( to the ``client`` stored on the current bucket. :type preserve_acl: bool - :param preserve_acl: DEPRECATED. (Optional) Copies ACL from old blob to - new blob. Default: True. + :param preserve_acl: (Optional) Copies ACL from old blob to new blob. + Default: True. :type source_generation: long :param source_generation: (Optional) The generation of the blob to be @@ -1307,13 +1307,8 @@ def copy_blob( timeout=timeout, ) - if preserve_acl is not None: - warnings.warn( - "preserve_acl arg is deprecated and will be removed in future." - "Do a subsequent ACL update instead.", - PendingDeprecationWarning, - stacklevel=1, - ) + if not preserve_acl: + new_blob.acl.save(acl={}, client=client, timeout=timeout) new_blob._set_properties(copy_result) return new_blob diff --git a/tests/unit/test_bucket.py b/tests/unit/test_bucket.py index 01eb8ec28..365e1f0e1 100644 --- a/tests/unit/test_bucket.py +++ b/tests/unit/test_bucket.py @@ -1144,31 +1144,29 @@ def test_copy_blobs_preserve_acl(self): dest = self._make_one(client=client, name=DEST) blob = self._make_blob(SOURCE, BLOB_NAME) - with mock.patch("warnings.warn") as warn: - new_blob = source.copy_blob( - blob, dest, NEW_NAME, client=client, preserve_acl=False - ) - warn.assert_called_once_with( - "preserve_acl arg is deprecated and will be removed in future." - "Do a subsequent ACL update instead.", - PendingDeprecationWarning, - stacklevel=1, - ) + 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 ) + NEW_BLOB_PATH = "/b/{}/o/{}".format(DEST, NEW_NAME) - self.assertEqual(connection._requested[0]["method"], "POST") - self.assertEqual(connection._requested[0]["path"], COPY_PATH) - self.assertEqual(connection._requested[0]["query_params"], {}) - self.assertEqual( - connection._requested[0]["timeout"], self._get_default_timeout() - ) + self.assertEqual(kw1["method"], "POST") + self.assertEqual(kw1["path"], COPY_PATH) + self.assertEqual(kw1["query_params"], {}) + self.assertEqual(kw1["timeout"], self._get_default_timeout()) + + self.assertEqual(kw2["method"], "PATCH") + self.assertEqual(kw2["path"], NEW_BLOB_PATH) + self.assertEqual(kw2["query_params"], {"projection": "full"}) + self.assertEqual(kw2["timeout"], self._get_default_timeout()) def test_copy_blobs_w_name_and_user_project(self): SOURCE = "source" From d43b68e65848bc7507853706ba1618b897db9723 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 17 Apr 2020 12:51:03 +0300 Subject: [PATCH 3/3] add deprecation warning into docs, provide a snippet of how to copy ACL --- google/cloud/storage/bucket.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/google/cloud/storage/bucket.py b/google/cloud/storage/bucket.py index c2a909357..118374c4d 100644 --- a/google/cloud/storage/bucket.py +++ b/google/cloud/storage/bucket.py @@ -1268,7 +1268,8 @@ def copy_blob( to the ``client`` stored on the current bucket. :type preserve_acl: bool - :param preserve_acl: (Optional) Copies ACL from old blob to new blob. + :param preserve_acl: DEPRECATED. This argument is not functional! + (Optional) Copies ACL from old blob to new blob. Default: True. :type source_generation: long @@ -1284,6 +1285,20 @@ def copy_blob( :rtype: :class:`google.cloud.storage.blob.Blob` :returns: The new Blob. + + Example: + Copy a blob including ACL. + + >>> from google.cloud import storage + + >>> client = storage.Client(project="project") + + >>> bucket = client.bucket("bucket") + >>> dst_bucket = client.bucket("destination-bucket") + + >>> blob = bucket.blob("file.ext") + >>> new_blob = bucket.copy_blob(blob, dst_bucket) + >>> new_blob.acl.save(blob.acl) """ client = self._require_client(client) query_params = {}