From 5c3f236d748d65619eabb8bd7dd3f753023d6a9e Mon Sep 17 00:00:00 2001 From: HemangChothani Date: Wed, 17 Jun 2020 16:31:47 +0530 Subject: [PATCH 1/3] feat(storage): rename download_as_string to bytes and add new method --- google/cloud/storage/blob.py | 77 ++++++++++++++++++++++++++++++- tests/perf/benchwrapper.py | 2 +- tests/system/test_system.py | 89 ++++++++++++++++++++---------------- tests/unit/test_blob.py | 54 +++++++++++++++++++++- 4 files changed, 178 insertions(+), 44 deletions(-) diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index ec6c6b08e..92fc320b4 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -1036,7 +1036,7 @@ def download_to_filename( mtime = updated.timestamp() os.utime(file_obj.name, (mtime, mtime)) - def download_as_string( + def download_as_bytes( self, client=None, start=None, @@ -1049,6 +1049,10 @@ def download_as_string( ): """Download the contents of this blob as a bytes object. + .. note:: + The method only supports `python3`, for python2 it returns data + as a string. + If :attr:`user_project` is set on the bucket, bills the API request to that project. @@ -1106,6 +1110,77 @@ def download_as_string( ) return string_buffer.getvalue() + def download_as_string( + self, + client=None, + start=None, + end=None, + raw_download=False, + if_generation_match=None, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + ): + """Download the contents of this blob as a string. + + If :attr:`user_project` is set on the bucket, bills the API request + to that project. + + :type client: :class:`~google.cloud.storage.client.Client` or + ``NoneType`` + :param client: (Optional) The client to use. If not passed, falls back + to the ``client`` stored on the blob's bucket. + + :type start: int + :param start: (Optional) The first byte in a range to be downloaded. + + :type end: int + :param end: (Optional) The last byte in a range to be downloaded. + + :type raw_download: bool + :param raw_download: + (Optional) If true, download the object without any expansion. + + :type if_generation_match: long + :param if_generation_match: (Optional) Make the operation conditional on whether + the blob's current generation matches the given value. + Setting to 0 makes the operation succeed only if there + are no live versions of the blob. + + :type if_generation_not_match: long + :param if_generation_not_match: (Optional) Make the operation conditional on whether + the blob's current generation does not match the given + value. If no live blob exists, the precondition fails. + Setting to 0 makes the operation succeed only if there + is a live version of the blob. + + :param if_metageneration_match: (Optional) Make the operation conditional on whether the + blob's current metageneration matches the given value. + + :type if_metageneration_not_match: long + :param if_metageneration_not_match: (Optional) Make the operation conditional on whether the + blob's current metageneration does not match the given value. + + :rtype: str + :returns: The data stored in this blob. + + :raises: :class:`google.cloud.exceptions.NotFound` + """ + data = self.download_as_bytes( + client=client, + start=start, + end=end, + raw_download=raw_download, + if_generation_match=if_generation_match, + if_generation_not_match=if_generation_not_match, + if_metageneration_match=if_metageneration_match, + if_metageneration_not_match=if_metageneration_not_match, + ) + if six.PY2: + return data + else: + return data.decode() + def _get_content_type(self, content_type, filename=None): """Determine the content type from the current object. diff --git a/tests/perf/benchwrapper.py b/tests/perf/benchwrapper.py index c81d6bb20..a92cd5f6a 100644 --- a/tests/perf/benchwrapper.py +++ b/tests/perf/benchwrapper.py @@ -35,7 +35,7 @@ def Write(self, request, context): def Read(self, request, context): bucket = client.bucket(request.bucketName) blob = storage.Blob(request.objectName, bucket) - blob.download_as_string() + blob.download_as_bytes() return storage_pb2.EmptyResponse() diff --git a/tests/system/test_system.py b/tests/system/test_system.py index 2afc1e515..ecf817d06 100644 --- a/tests/system/test_system.py +++ b/tests/system/test_system.py @@ -420,8 +420,8 @@ def test_copy_existing_file_with_user_project(self): ) to_delete.append(new_blob) - base_contents = blob.download_as_string() - copied_contents = new_blob.download_as_string() + base_contents = blob.download_as_bytes() + copied_contents = new_blob.download_as_bytes() self.assertEqual(base_contents, copied_contents) finally: for blob in to_delete: @@ -450,8 +450,8 @@ def test_copy_file_with_generation_match(self): ) to_delete.append(new_blob) - base_contents = blob.download_as_string() - copied_contents = new_blob.download_as_string() + base_contents = blob.download_as_bytes() + copied_contents = new_blob.download_as_bytes() self.assertEqual(base_contents, copied_contents) finally: for blob in to_delete: @@ -480,8 +480,8 @@ def test_copy_file_with_metageneration_match(self): ) to_delete.append(new_blob) - base_contents = blob.download_as_string() - copied_contents = new_blob.download_as_string() + base_contents = blob.download_as_bytes() + copied_contents = new_blob.download_as_bytes() self.assertEqual(base_contents, copied_contents) finally: for blob in to_delete: @@ -507,7 +507,7 @@ def test_bucket_get_blob_with_user_project(self): to_add.upload_from_string(data) try: found = with_user_project.get_blob("blob-name") - self.assertEqual(found.download_as_string(), data) + self.assertEqual(found.download_as_bytes(), data) finally: to_add.delete() @@ -622,8 +622,8 @@ def test_crud_blob_w_user_project(self): blob.reload() # Exercise 'objects.get' (media) w/ userProject. - self.assertEqual(blob0.download_as_string(), file_contents) - self.assertEqual(blob1.download_as_string(), b"gen1") + self.assertEqual(blob0.download_as_bytes(), file_contents) + self.assertEqual(blob1.download_as_bytes(), b"gen1") # Exercise 'objects.patch' w/ userProject. blob0.content_language = "en" @@ -683,10 +683,10 @@ def test_crud_blob_w_generation_match(self): # Exercise 'objects.get' (media) w/ generation match. self.assertEqual( - blob0.download_as_string(if_generation_match=gen0), file_contents + blob0.download_as_bytes(if_generation_match=gen0), file_contents ) self.assertEqual( - blob1.download_as_string(if_generation_not_match=gen0), b"gen1" + blob1.download_as_bytes(if_generation_not_match=gen0), b"gen1" ) # Exercise 'objects.patch' w/ generation match. @@ -824,8 +824,8 @@ def test_copy_existing_file(self): ) self.case_blobs_to_delete.append(new_blob) - base_contents = blob.download_as_string() - copied_contents = new_blob.download_as_string() + base_contents = blob.download_as_bytes() + copied_contents = new_blob.download_as_bytes() self.assertEqual(base_contents, copied_contents) def test_download_blob_w_uri(self): @@ -845,6 +845,15 @@ def test_download_blob_w_uri(self): self.assertEqual(file_contents, stored_contents) + def test_download_blob_as_string(self): + blob = self.bucket.blob("MyBuffer") + file_contents = "Hello World" + blob.upload_from_string(file_contents) + self.case_blobs_to_delete.append(blob) + + stored_contents = blob.download_as_string() + self.assertEqual(file_contents, stored_contents) + def test_upload_gzip_encoded_download_raw(self): payload = b"DEADBEEF" * 1000 raw_stream = io.BytesIO() @@ -856,10 +865,10 @@ def test_upload_gzip_encoded_download_raw(self): blob.content_encoding = "gzip" blob.upload_from_file(raw_stream, rewind=True) - expanded = blob.download_as_string() + expanded = blob.download_as_bytes() self.assertEqual(expanded, payload) - raw = blob.download_as_string(raw_download=True) + raw = blob.download_as_bytes(raw_download=True) self.assertEqual(raw, zipped) def test_resumable_upload_with_generation_match(self): @@ -919,7 +928,7 @@ def test_fetch_object_and_check_content(self): for blob_name, file_contents in test_data.items(): blob = bucket.blob(blob_name) self.assertEqual(blob.name, blob_name) - self.assertEqual(blob.download_as_string(), file_contents) + self.assertEqual(blob.download_as_bytes(), file_contents) class TestStorageListFiles(TestStorageFiles): @@ -1401,7 +1410,7 @@ def test_compose_create_new_blob(self): destination.compose([source_1, source_2]) self.case_blobs_to_delete.append(destination) - composed = destination.download_as_string() + composed = destination.download_as_bytes() self.assertEqual(composed, SOURCE_1 + SOURCE_2) def test_compose_create_new_blob_wo_content_type(self): @@ -1421,7 +1430,7 @@ def test_compose_create_new_blob_wo_content_type(self): self.case_blobs_to_delete.append(destination) self.assertIsNone(destination.content_type) - composed = destination.download_as_string() + composed = destination.download_as_bytes() self.assertEqual(composed, SOURCE_1 + SOURCE_2) def test_compose_replace_existing_blob(self): @@ -1438,7 +1447,7 @@ def test_compose_replace_existing_blob(self): original.compose([original, to_append]) - composed = original.download_as_string() + composed = original.download_as_bytes() self.assertEqual(composed, BEFORE + TO_APPEND) def test_compose_with_generation_match(self): @@ -1466,7 +1475,7 @@ def test_compose_with_generation_match(self): if_metageneration_match=[original.metageneration, to_append.metageneration], ) - composed = original.download_as_string() + composed = original.download_as_bytes() self.assertEqual(composed, BEFORE + TO_APPEND) @unittest.skipUnless(USER_PROJECT, "USER_PROJECT not set in environment.") @@ -1492,7 +1501,7 @@ def test_compose_with_user_project(self): destination.content_type = "text/plain" destination.compose([source_1, source_2]) - composed = destination.download_as_string() + composed = destination.download_as_bytes() self.assertEqual(composed, SOURCE_1 + SOURCE_2) finally: retry_429_harder(created.delete)(force=True) @@ -1508,7 +1517,7 @@ def test_rewrite_create_new_blob_add_encryption_key(self): source = self.bucket.blob("source") source.upload_from_filename(file_data["path"]) self.case_blobs_to_delete.append(source) - source_data = source.download_as_string() + source_data = source.download_as_bytes() KEY = os.urandom(32) dest = self.bucket.blob("dest", encryption_key=KEY) @@ -1519,7 +1528,7 @@ def test_rewrite_create_new_blob_add_encryption_key(self): self.assertEqual(rewritten, len(source_data)) self.assertEqual(total, len(source_data)) - self.assertEqual(source.download_as_string(), dest.download_as_string()) + self.assertEqual(source.download_as_bytes(), dest.download_as_bytes()) def test_rewrite_rotate_encryption_key(self): BLOB_NAME = "rotating-keys" @@ -1529,7 +1538,7 @@ def test_rewrite_rotate_encryption_key(self): source = self.bucket.blob(BLOB_NAME, encryption_key=SOURCE_KEY) source.upload_from_filename(file_data["path"]) self.case_blobs_to_delete.append(source) - source_data = source.download_as_string() + source_data = source.download_as_bytes() DEST_KEY = os.urandom(32) dest = self.bucket.blob(BLOB_NAME, encryption_key=DEST_KEY) @@ -1541,7 +1550,7 @@ def test_rewrite_rotate_encryption_key(self): self.assertEqual(rewritten, len(source_data)) self.assertEqual(total, len(source_data)) - self.assertEqual(dest.download_as_string(), source_data) + self.assertEqual(dest.download_as_bytes(), source_data) @unittest.skipUnless(USER_PROJECT, "USER_PROJECT not set in environment.") def test_rewrite_add_key_with_user_project(self): @@ -1557,7 +1566,7 @@ def test_rewrite_add_key_with_user_project(self): source = with_user_project.blob("source") source.upload_from_filename(file_data["path"]) - source_data = source.download_as_string() + source_data = source.download_as_bytes() KEY = os.urandom(32) dest = with_user_project.blob("dest", encryption_key=KEY) @@ -1567,7 +1576,7 @@ def test_rewrite_add_key_with_user_project(self): self.assertEqual(rewritten, len(source_data)) self.assertEqual(total, len(source_data)) - self.assertEqual(source.download_as_string(), dest.download_as_string()) + self.assertEqual(source.download_as_bytes(), dest.download_as_bytes()) finally: retry_429_harder(created.delete)(force=True) @@ -1587,7 +1596,7 @@ def test_rewrite_rotate_with_user_project(self): SOURCE_KEY = os.urandom(32) source = with_user_project.blob(BLOB_NAME, encryption_key=SOURCE_KEY) source.upload_from_filename(file_data["path"]) - source_data = source.download_as_string() + source_data = source.download_as_bytes() DEST_KEY = os.urandom(32) dest = with_user_project.blob(BLOB_NAME, encryption_key=DEST_KEY) @@ -1597,7 +1606,7 @@ def test_rewrite_rotate_with_user_project(self): self.assertEqual(rewritten, len(source_data)) self.assertEqual(total, len(source_data)) - self.assertEqual(dest.download_as_string(), source_data) + self.assertEqual(dest.download_as_bytes(), source_data) finally: retry_429_harder(created.delete)(force=True) @@ -1613,7 +1622,7 @@ def test_rewrite_with_generation_match(self): source = bucket.blob(BLOB_NAME) source.upload_from_filename(file_data["path"]) - source_data = source.download_as_string() + source_data = source.download_as_bytes() dest = bucket.blob(BLOB_NAME) @@ -1631,7 +1640,7 @@ def test_rewrite_with_generation_match(self): self.assertEqual(token, None) self.assertEqual(rewritten, len(source_data)) self.assertEqual(total, len(source_data)) - self.assertEqual(dest.download_as_string(), source_data) + self.assertEqual(dest.download_as_bytes(), source_data) finally: retry_429_harder(created.delete)(force=True) @@ -1898,7 +1907,7 @@ def test_blob_w_explicit_kms_key_name(self): blob.upload_from_filename(file_data["path"]) self.case_blobs_to_delete.append(blob) with open(file_data["path"], "rb") as _file_data: - self.assertEqual(blob.download_as_string(), _file_data.read()) + self.assertEqual(blob.download_as_bytes(), _file_data.read()) # We don't know the current version of the key. self.assertTrue(blob.kms_key_name.startswith(kms_key_name)) @@ -1925,7 +1934,7 @@ def test_bucket_w_default_kms_key_name(self): defaulted_blob.upload_from_filename(file_data["path"]) self.case_blobs_to_delete.append(defaulted_blob) - self.assertEqual(defaulted_blob.download_as_string(), contents) + self.assertEqual(defaulted_blob.download_as_bytes(), contents) # We don't know the current version of the key. self.assertTrue(defaulted_blob.kms_key_name.startswith(kms_key_name)) @@ -1937,7 +1946,7 @@ def test_bucket_w_default_kms_key_name(self): override_blob.upload_from_filename(file_data["path"]) self.case_blobs_to_delete.append(override_blob) - self.assertEqual(override_blob.download_as_string(), contents) + self.assertEqual(override_blob.download_as_bytes(), contents) # We don't know the current version of the key. self.assertTrue(override_blob.kms_key_name.startswith(alt_kms_key_name)) @@ -1948,7 +1957,7 @@ def test_bucket_w_default_kms_key_name(self): alt_blob.upload_from_filename(file_data["path"]) self.case_blobs_to_delete.append(alt_blob) - self.assertEqual(alt_blob.download_as_string(), contents) + self.assertEqual(alt_blob.download_as_bytes(), contents) # We don't know the current version of the key. self.assertTrue(alt_blob.kms_key_name.startswith(alt_kms_key_name)) @@ -1959,7 +1968,7 @@ def test_bucket_w_default_kms_key_name(self): cleartext_blob.upload_from_filename(file_data["path"]) self.case_blobs_to_delete.append(cleartext_blob) - self.assertEqual(cleartext_blob.download_as_string(), contents) + self.assertEqual(cleartext_blob.download_as_bytes(), contents) self.assertIsNone(cleartext_blob.kms_key_name) def test_rewrite_rotate_csek_to_cmek(self): @@ -1970,7 +1979,7 @@ def test_rewrite_rotate_csek_to_cmek(self): source = self.bucket.blob(BLOB_NAME, encryption_key=SOURCE_KEY) source.upload_from_filename(file_data["path"]) self.case_blobs_to_delete.append(source) - source_data = source.download_as_string() + source_data = source.download_as_bytes() kms_key_name = self._kms_key_name() @@ -1992,7 +2001,7 @@ def test_rewrite_rotate_csek_to_cmek(self): self.assertEqual(rewritten, len(source_data)) self.assertEqual(total, len(source_data)) - self.assertEqual(dest.download_as_string(), source_data) + self.assertEqual(dest.download_as_bytes(), source_data) def test_upload_new_blob_w_bucket_cmek_enabled(self): blob_name = "test-blob" @@ -2012,7 +2021,7 @@ def test_upload_new_blob_w_bucket_cmek_enabled(self): blob.upload_from_string(alt_payload, if_generation_match=blob.generation) self.case_blobs_to_delete.append(blob) - self.assertEqual(blob.download_as_string(), alt_payload) + self.assertEqual(blob.download_as_bytes(), alt_payload) self.bucket.default_kms_key_name = None self.bucket.patch() @@ -2240,7 +2249,7 @@ def test_new_bucket_w_ubla(self): blob.upload_from_string(payload) found = bucket.get_blob(blob_name) - self.assertEqual(found.download_as_string(), payload) + self.assertEqual(found.download_as_bytes(), payload) blob_acl = blob.acl with self.assertRaises(exceptions.BadRequest): diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py index 001f8801f..0f3ef73ca 100644 --- a/tests/unit/test_blob.py +++ b/tests/unit/test_blob.py @@ -1305,6 +1305,56 @@ def test_download_to_filename_w_key(self): stream = blob._do_download.mock_calls[0].args[1] self.assertEqual(stream.name, temp.name) + def _download_as_bytes_helper(self, raw_download): + blob_name = "blob-name" + client = mock.Mock(spec=["_http"]) + bucket = _Bucket(client) + media_link = "http://example.com/media/" + properties = {"mediaLink": media_link} + blob = self._make_one(blob_name, bucket=bucket, properties=properties) + blob._do_download = mock.Mock() + + fetched = blob.download_as_bytes(raw_download=raw_download) + self.assertEqual(fetched, b"") + + headers = {"accept-encoding": "gzip"} + blob._do_download.assert_called_once_with( + client._http, mock.ANY, media_link, headers, None, None, raw_download + ) + stream = blob._do_download.mock_calls[0].args[1] + self.assertIsInstance(stream, io.BytesIO) + + def test_download_as_bytes_w_generation_match(self): + GENERATION_NUMBER = 6 + MEDIA_LINK = "http://example.com/media/" + + client = mock.Mock(spec=["_http"]) + blob = self._make_one( + "blob-name", bucket=_Bucket(client), properties={"mediaLink": MEDIA_LINK} + ) + blob.download_to_file = mock.Mock() + + fetched = blob.download_as_bytes(if_generation_match=GENERATION_NUMBER) + self.assertEqual(fetched, b"") + + blob.download_to_file.assert_called_once_with( + mock.ANY, + client=None, + start=None, + end=None, + raw_download=False, + if_generation_match=GENERATION_NUMBER, + if_generation_not_match=None, + if_metageneration_match=None, + if_metageneration_not_match=None, + ) + + def test_download_as_bytes_wo_raw(self): + self._download_as_bytes_helper(raw_download=False) + + def test_download_as_bytes_w_raw(self): + self._download_as_bytes_helper(raw_download=True) + def _download_as_string_helper(self, raw_download): blob_name = "blob-name" client = mock.Mock(spec=["_http"]) @@ -1315,7 +1365,7 @@ def _download_as_string_helper(self, raw_download): blob._do_download = mock.Mock() fetched = blob.download_as_string(raw_download=raw_download) - self.assertEqual(fetched, b"") + self.assertEqual(fetched, "") headers = {"accept-encoding": "gzip"} blob._do_download.assert_called_once_with( @@ -1335,7 +1385,7 @@ def test_download_as_string_w_generation_match(self): blob.download_to_file = mock.Mock() fetched = blob.download_as_string(if_generation_match=GENERATION_NUMBER) - self.assertEqual(fetched, b"") + self.assertEqual(fetched, "") blob.download_to_file.assert_called_once_with( mock.ANY, From cc5f3650d3d23674bdac9eb79e32eb8785bf80a3 Mon Sep 17 00:00:00 2001 From: HemangChothani Date: Fri, 24 Jul 2020 18:22:59 +0530 Subject: [PATCH 2/3] feat(storage): add deprecated warning for dowload_as_string --- google/cloud/storage/blob.py | 16 ++++++++++++++-- tests/unit/test_blob.py | 10 +++++++++- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index 2e1870b0d..2b7e29fa0 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -1163,11 +1163,14 @@ def download_as_string( if_metageneration_not_match=None, timeout=_DEFAULT_TIMEOUT, ): - """Download the contents of this blob as a bytes object. + """(Deprecated) Download the contents of this blob as a bytes object. If :attr:`user_project` is set on the bucket, bills the API request to that project. + .. note:: + Deprecated alias for :meth:`download_as_bytes`. + :type client: :class:`~google.cloud.storage.client.Client` or ``NoneType`` :param client: (Optional) The client to use. If not passed, falls back @@ -1216,6 +1219,12 @@ def download_as_string( :raises: :class:`google.cloud.exceptions.NotFound` """ + warnings.warn( + "Blob.download_as_string() is deprecated and will be removed in future." + "Use Blob.download_as_bytes() instead.", + PendingDeprecationWarning, + stacklevel=1, + ) return self.download_as_bytes( client=client, start=start, @@ -1311,7 +1320,10 @@ def download_as_text( timeout=timeout, ) if six.PY2: - return data + if self.content_encoding: + return data.decode(self.content_encoding) + else: + return data.decode(encoding) else: if self.content_encoding: return data.decode(self.content_encoding) diff --git a/tests/unit/test_blob.py b/tests/unit/test_blob.py index 924e8bdf2..4ac2c712b 100644 --- a/tests/unit/test_blob.py +++ b/tests/unit/test_blob.py @@ -1555,7 +1555,8 @@ def test_download_as_text_w_custom_timeout(self): def test_download_as_text_w_encoding(self): self._download_as_text_helper(raw_download=False, encoding="utf-8") - def test_download_as_string(self): + @mock.patch("warnings.warn") + def test_download_as_string(self, mock_warn): MEDIA_LINK = "http://example.com/media/" client = mock.Mock(spec=["_http"]) @@ -1580,6 +1581,13 @@ def test_download_as_string(self): timeout=self._get_default_timeout(), ) + mock_warn.assert_called_with( + "Blob.download_as_string() is deprecated and will be removed in future." + "Use Blob.download_as_bytes() instead.", + PendingDeprecationWarning, + stacklevel=1, + ) + def test__get_content_type_explicit(self): blob = self._make_one(u"blob-name", bucket=None) From 650e39fa1e5d43489a95da9594854d49f34fd10b Mon Sep 17 00:00:00 2001 From: HemangChothani Date: Wed, 5 Aug 2020 11:37:02 +0530 Subject: [PATCH 3/3] feat(storage): remove condition for python2 --- google/cloud/storage/blob.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/google/cloud/storage/blob.py b/google/cloud/storage/blob.py index 120f5e956..1380f41bb 100644 --- a/google/cloud/storage/blob.py +++ b/google/cloud/storage/blob.py @@ -1348,16 +1348,11 @@ def download_as_text( if_metageneration_not_match=if_metageneration_not_match, timeout=timeout, ) - if six.PY2: - if self.content_encoding: - return data.decode(self.content_encoding) - else: - return data.decode(encoding) + + if self.content_encoding: + return data.decode(self.content_encoding) else: - if self.content_encoding: - return data.decode(self.content_encoding) - else: - return data.decode(encoding) + return data.decode(encoding) def _get_content_type(self, content_type, filename=None): """Determine the content type from the current object.