From ad54362ca6bb33fc4cebcae05abe1be08a08a3ee Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 14 Jun 2019 16:22:40 +0300 Subject: [PATCH 1/8] Union create-or-use bucket snippets into one helper --- storage/google/cloud/storage/client.py | 36 ++++++++++++++++---------- storage/tests/unit/test_client.py | 4 +++ 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/storage/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index 7452d7adcd2b..17462f6962b5 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -153,6 +153,26 @@ def _pop_batch(self): """ return self._batch_stack.pop() + def _use_or_init_bucket(self, bucket_or_name): + """Helper to return given bucket or create new by name. + + Args: + bucket_or_name (Union[ \ + :class:`~google.cloud.storage.bucket.Bucket`, \ + str, \ + ]): + The bucket resource to pass or name to create. + + Returns: + google.cloud.storage.bucket.Bucket + The newly created bucket. + """ + if isinstance(bucket_or_name, Bucket): + bucket = bucket_or_name + else: + bucket = Bucket(self, name=bucket_or_name) + return bucket + @property def current_batch(self): """Currently-active batch. @@ -252,12 +272,7 @@ def get_bucket(self, bucket_or_name): >>> bucket = client.get_bucket(bucket) # API request. """ - - bucket = None - if isinstance(bucket_or_name, Bucket): - bucket = bucket_or_name - else: - bucket = Bucket(self, name=bucket_or_name) + bucket = self._use_or_init_bucket(bucket_or_name) bucket.reload(client=self) return bucket @@ -299,7 +314,7 @@ def create_bucket(self, bucket_or_name, requester_pays=None, project=None): Optional. Whether requester pays for API requests for this bucket and its blobs. project (str): - Optional. the project under which the bucket is to be created. + Optional. the project under which the bucket is to be created. If not passed, uses the project set on the client. Returns: @@ -331,12 +346,7 @@ def create_bucket(self, bucket_or_name, requester_pays=None, project=None): >>> bucket = client.create_bucket(bucket) # API request. """ - - bucket = None - if isinstance(bucket_or_name, Bucket): - bucket = bucket_or_name - else: - bucket = Bucket(self, name=bucket_or_name) + bucket = self._use_or_init_bucket(bucket_or_name) if requester_pays is not None: bucket.requester_pays = requester_pays diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index e874b44e1241..5ccf89f40a71 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -821,3 +821,7 @@ def dummy_response(): self.assertEqual(page.remaining, 0) self.assertIsInstance(bucket, Bucket) self.assertEqual(bucket.name, blob_name) + + +if __name__ == '__main__': + unittest.main() From 183533768d9031f3f36f311f594bef998ba8ee47 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Fri, 14 Jun 2019 17:22:03 +0300 Subject: [PATCH 2/8] Deleted test_list_blobs - looks like it's a full copy of test_list_blobs_defaults --- storage/tests/unit/test_bucket.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/storage/tests/unit/test_bucket.py b/storage/tests/unit/test_bucket.py index 31b7d293010e..2f0661537a09 100644 --- a/storage/tests/unit/test_bucket.py +++ b/storage/tests/unit/test_bucket.py @@ -782,19 +782,6 @@ def test_list_blobs_w_all_arguments_and_user_project(self): self.assertEqual(kw["path"], "/b/%s/o" % NAME) self.assertEqual(kw["query_params"], EXPECTED) - def test_list_blobs(self): - NAME = "name" - connection = _Connection({"items": []}) - client = _Client(connection) - bucket = self._make_one(client=client, name=NAME) - iterator = bucket.list_blobs() - blobs = list(iterator) - self.assertEqual(blobs, []) - kw, = connection._requested - self.assertEqual(kw["method"], "GET") - self.assertEqual(kw["path"], "/b/%s/o" % NAME) - self.assertEqual(kw["query_params"], {"projection": "noAcl"}) - def test_list_notifications(self): from google.cloud.storage.notification import BucketNotification from google.cloud.storage.notification import _TOPIC_REF_FMT From e038d1940c20d02cdbad16381841a506573757db Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 17 Jun 2019 11:05:57 +0300 Subject: [PATCH 3/8] Add new method client.list_blobs that's the same as bucket.list_blobs() --- storage/google/cloud/storage/client.py | 76 +++++++++++++++++ storage/tests/unit/test_client.py | 111 +++++++++++++++++++++++++ 2 files changed, 187 insertions(+) diff --git a/storage/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index 17462f6962b5..9923ba69c3a6 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -404,6 +404,82 @@ def download_blob_to_file(self, blob_or_uri, file_obj, start=None, end=None): blob_or_uri.download_to_file(file_obj, client=self, start=start, end=end) + def list_blobs( + self, + bucket_or_name, + max_results=None, + page_token=None, + prefix=None, + delimiter=None, + versions=None, + projection="noAcl", + fields=None, + ): + """Return an iterator used to find blobs in the bucket. + + If :attr:`user_project` is set, bills the API request to that project. + + bucket_or_name (Union[ \ + :class:`~google.cloud.storage.bucket.Bucket`, \ + str, \ + ]): + The bucket resource to pass or name to create. + + :type max_results: int + :param max_results: + (Optional) The maximum number of blobs in each page of results + from this request. Non-positive values are ignored. Defaults to + a sensible value set by the API. + + :type page_token: str + :param page_token: + (Optional) If present, return the next batch of blobs, using the + value, which must correspond to the ``nextPageToken`` value + returned in the previous response. Deprecated: use the ``pages`` + property of the returned iterator instead of manually passing the + token. + + :type prefix: str + :param prefix: (Optional) prefix used to filter blobs. + + :type delimiter: str + :param delimiter: (Optional) Delimiter, used with ``prefix`` to + emulate hierarchy. + + :type versions: bool + :param versions: (Optional) Whether object versions should be returned + as separate blobs. + + :type projection: str + :param projection: (Optional) If used, must be 'full' or 'noAcl'. + Defaults to ``'noAcl'``. Specifies the set of + properties to return. + + :type fields: str + :param fields: + (Optional) Selector specifying which fields to include + in a partial response. Must be a list of fields. For + example to get a partial response with just the next + page token and the name and language of each blob returned: + ``'items(name,contentLanguage),nextPageToken'``. + See: https://cloud.google.com/storage/docs/json_api/v1/parameters#fields + + :rtype: :class:`~google.api_core.page_iterator.Iterator` + :returns: Iterator of all :class:`~google.cloud.storage.blob.Blob` + in this bucket matching the arguments. + """ + bucket = self._use_or_init_bucket(bucket_or_name) + return bucket.list_blobs( + max_results=max_results, + page_token=page_token, + prefix=prefix, + delimiter=delimiter, + versions=versions, + projection=projection, + fields=fields, + client=self + ) + def list_buckets( self, max_results=None, diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index 5ccf89f40a71..c7a142807f2b 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -639,6 +639,81 @@ def test_download_blob_to_file_with_invalid_uri(self): "http://bucket_name/path/to/object", file_obj ) + def test_list_blobs(self): + from google.cloud.storage.bucket import Bucket + BUCKET_NAME = "bucket-name" + + credentials = _make_credentials() + client = self._make_one(project="PROJECT", credentials=credentials) + connection = _Connection({"items": []}) + + with mock.patch( + 'google.cloud.storage.client.Client._connection', + new_callable=mock.PropertyMock + ) as client_mock: + client_mock.return_value = connection + + bucket_obj = Bucket(client, BUCKET_NAME) + iterator = client.list_blobs(bucket_obj) + blobs = list(iterator) + + self.assertEqual(blobs, []) + kw, = connection._requested + self.assertEqual(kw["method"], "GET") + self.assertEqual(kw["path"], "/b/%s/o" % BUCKET_NAME) + self.assertEqual(kw["query_params"], {"projection": "noAcl"}) + + def test_list_blobs_w_all_arguments_and_user_project(self): + from google.cloud.storage.bucket import Bucket + BUCKET_NAME = "name" + USER_PROJECT = "user-project-123" + MAX_RESULTS = 10 + PAGE_TOKEN = "ABCD" + PREFIX = "subfolder" + DELIMITER = "/" + VERSIONS = True + PROJECTION = "full" + FIELDS = "items/contentLanguage,nextPageToken" + EXPECTED = { + "maxResults": 10, + "pageToken": PAGE_TOKEN, + "prefix": PREFIX, + "delimiter": DELIMITER, + "versions": VERSIONS, + "projection": PROJECTION, + "fields": FIELDS, + "userProject": USER_PROJECT, + } + + credentials = _make_credentials() + client = self._make_one(project=USER_PROJECT, credentials=credentials) + connection = _Connection({"items": []}) + + with mock.patch( + 'google.cloud.storage.client.Client._connection', + new_callable=mock.PropertyMock + ) as client_mock: + client_mock.return_value = connection + + bucket = Bucket(client, BUCKET_NAME, user_project=USER_PROJECT) + iterator = client.list_blobs( + bucket_or_name=bucket, + max_results=MAX_RESULTS, + page_token=PAGE_TOKEN, + prefix=PREFIX, + delimiter=DELIMITER, + versions=VERSIONS, + projection=PROJECTION, + fields=FIELDS, + ) + blobs = list(iterator) + + self.assertEqual(blobs, []) + kw, = connection._requested + self.assertEqual(kw["method"], "GET") + self.assertEqual(kw["path"], "/b/%s/o" % BUCKET_NAME) + self.assertEqual(kw["query_params"], EXPECTED) + def test_list_buckets_wo_project(self): CREDENTIALS = _make_credentials() client = self._make_one(project=None, credentials=CREDENTIALS) @@ -823,5 +898,41 @@ def dummy_response(): self.assertEqual(bucket.name, blob_name) +class _Connection(object): + _delete_bucket = False + + def __init__(self, *responses): + self._responses = responses + self._requested = [] + self._deleted_buckets = [] + self.credentials = None + + @staticmethod + def _is_bucket_path(path): + # Now just ensure the path only has /b/ and one more segment. + return path.startswith("/b/") and path.count("/") == 2 + + def api_request(self, **kw): + from google.cloud.exceptions import NotFound + + self._requested.append(kw) + + method = kw.get("method") + path = kw.get("path", "") + if method == "DELETE" and self._is_bucket_path(path): + self._deleted_buckets.append(kw) + if self._delete_bucket: + return + else: + raise NotFound("miss") + + try: + response, self._responses = self._responses[0], self._responses[1:] + except IndexError: + raise NotFound("miss") + else: + return response + + if __name__ == '__main__': unittest.main() From e120a23809e9c7eb192c93246703a53192316951 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 17 Jun 2019 11:11:53 +0300 Subject: [PATCH 4/8] Redact comment and delete unittest runner --- storage/google/cloud/storage/client.py | 2 +- storage/tests/unit/test_client.py | 4 ---- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/storage/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index 9923ba69c3a6..734ca23f6c4e 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -165,7 +165,7 @@ def _use_or_init_bucket(self, bucket_or_name): Returns: google.cloud.storage.bucket.Bucket - The newly created bucket. + The newly created bucket or the given one. """ if isinstance(bucket_or_name, Bucket): bucket = bucket_or_name diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index c7a142807f2b..9044ee8a347c 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -932,7 +932,3 @@ def api_request(self, **kw): raise NotFound("miss") else: return response - - -if __name__ == '__main__': - unittest.main() From 81757888c29a1600cdcbfcbd9c113f9914943a08 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 17 Jun 2019 12:18:41 +0300 Subject: [PATCH 5/8] Add deprecation for bucket.list_blobs --- storage/google/cloud/storage/bucket.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index f3097a5f7eda..579817df8158 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -770,6 +770,11 @@ def list_blobs( :returns: Iterator of all :class:`~google.cloud.storage.blob.Blob` in this bucket matching the arguments. """ + warnings.warn( + "Bucket.list_blobs method is deprecated. Use Client.list_blobs instead.", + PendingDeprecationWarning, + stacklevel=2, + ) extra_params = {"projection": projection} if prefix is not None: From 7e201311c4c9989331c5b35ee9fbc811f2470e53 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 17 Jun 2019 13:55:43 +0300 Subject: [PATCH 6/8] Changed warn to comment --- storage/google/cloud/storage/bucket.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/storage/google/cloud/storage/bucket.py b/storage/google/cloud/storage/bucket.py index 579817df8158..81f03e190539 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -721,6 +721,9 @@ def list_blobs( ): """Return an iterator used to find blobs in the bucket. + .. note:: + Direct use of this method is deprecated. Use Client.list_blobs instead. + If :attr:`user_project` is set, bills the API request to that project. :type max_results: int @@ -770,11 +773,6 @@ def list_blobs( :returns: Iterator of all :class:`~google.cloud.storage.blob.Blob` in this bucket matching the arguments. """ - warnings.warn( - "Bucket.list_blobs method is deprecated. Use Client.list_blobs instead.", - PendingDeprecationWarning, - stacklevel=2, - ) extra_params = {"projection": projection} if prefix is not None: From f0831566ba0f3f57a08542bc592f46e83a4575d3 Mon Sep 17 00:00:00 2001 From: IlyaFaer Date: Mon, 24 Jun 2019 13:25:14 +0300 Subject: [PATCH 7/8] Bring method docs to Google-style, use mock connection instead of class --- storage/google/cloud/storage/client.py | 98 +++++++++++++------------- storage/tests/unit/test_client.py | 68 ++++++------------ 2 files changed, 70 insertions(+), 96 deletions(-) diff --git a/storage/google/cloud/storage/client.py b/storage/google/cloud/storage/client.py index 734ca23f6c4e..b41108532b02 100644 --- a/storage/google/cloud/storage/client.py +++ b/storage/google/cloud/storage/client.py @@ -153,7 +153,7 @@ def _pop_batch(self): """ return self._batch_stack.pop() - def _use_or_init_bucket(self, bucket_or_name): + def _bucket_arg_to_bucket(self, bucket_or_name): """Helper to return given bucket or create new by name. Args: @@ -272,7 +272,7 @@ def get_bucket(self, bucket_or_name): >>> bucket = client.get_bucket(bucket) # API request. """ - bucket = self._use_or_init_bucket(bucket_or_name) + bucket = self._bucket_arg_to_bucket(bucket_or_name) bucket.reload(client=self) return bucket @@ -346,7 +346,7 @@ def create_bucket(self, bucket_or_name, requester_pays=None, project=None): >>> bucket = client.create_bucket(bucket) # API request. """ - bucket = self._use_or_init_bucket(bucket_or_name) + bucket = self._bucket_arg_to_bucket(bucket_or_name) if requester_pays is not None: bucket.requester_pays = requester_pays @@ -419,56 +419,54 @@ def list_blobs( If :attr:`user_project` is set, bills the API request to that project. - bucket_or_name (Union[ \ - :class:`~google.cloud.storage.bucket.Bucket`, \ - str, \ - ]): - The bucket resource to pass or name to create. - - :type max_results: int - :param max_results: - (Optional) The maximum number of blobs in each page of results - from this request. Non-positive values are ignored. Defaults to - a sensible value set by the API. - - :type page_token: str - :param page_token: - (Optional) If present, return the next batch of blobs, using the - value, which must correspond to the ``nextPageToken`` value - returned in the previous response. Deprecated: use the ``pages`` - property of the returned iterator instead of manually passing the - token. - - :type prefix: str - :param prefix: (Optional) prefix used to filter blobs. - - :type delimiter: str - :param delimiter: (Optional) Delimiter, used with ``prefix`` to - emulate hierarchy. - - :type versions: bool - :param versions: (Optional) Whether object versions should be returned - as separate blobs. - - :type projection: str - :param projection: (Optional) If used, must be 'full' or 'noAcl'. - Defaults to ``'noAcl'``. Specifies the set of - properties to return. + Args: + bucket_or_name (Union[ \ + :class:`~google.cloud.storage.bucket.Bucket`, \ + str, \ + ]): + The bucket resource to pass or name to create. - :type fields: str - :param fields: - (Optional) Selector specifying which fields to include - in a partial response. Must be a list of fields. For - example to get a partial response with just the next - page token and the name and language of each blob returned: - ``'items(name,contentLanguage),nextPageToken'``. - See: https://cloud.google.com/storage/docs/json_api/v1/parameters#fields + max_results (int): + (Optional) The maximum number of blobs in each page of results + from this request. Non-positive values are ignored. Defaults to + a sensible value set by the API. + + page_token (str): + (Optional) If present, return the next batch of blobs, using the + value, which must correspond to the ``nextPageToken`` value + returned in the previous response. Deprecated: use the ``pages`` + property of the returned iterator instead of manually passing the + token. + + prefix (str): + (Optional) prefix used to filter blobs. + + delimiter (str): + (Optional) Delimiter, used with ``prefix`` to + emulate hierarchy. + + versions (bool): + (Optional) Whether object versions should be returned + as separate blobs. + + projection (str): + (Optional) If used, must be 'full' or 'noAcl'. + Defaults to ``'noAcl'``. Specifies the set of + properties to return. + + fields (str): + (Optional) Selector specifying which fields to include + in a partial response. Must be a list of fields. For + example to get a partial response with just the next + page token and the name and language of each blob returned: + ``'items(name,contentLanguage),nextPageToken'``. + See: https://cloud.google.com/storage/docs/json_api/v1/parameters#fields - :rtype: :class:`~google.api_core.page_iterator.Iterator` - :returns: Iterator of all :class:`~google.cloud.storage.blob.Blob` - in this bucket matching the arguments. + Returns: + Iterator of all :class:`~google.cloud.storage.blob.Blob` + in this bucket matching the arguments. """ - bucket = self._use_or_init_bucket(bucket_or_name) + bucket = self._bucket_arg_to_bucket(bucket_or_name) return bucket.list_blobs( max_results=max_results, page_token=page_token, diff --git a/storage/tests/unit/test_client.py b/storage/tests/unit/test_client.py index 9044ee8a347c..89ed8570dbfa 100644 --- a/storage/tests/unit/test_client.py +++ b/storage/tests/unit/test_client.py @@ -28,6 +28,16 @@ def _make_credentials(): return mock.Mock(spec=google.auth.credentials.Credentials) +def _make_connection(*responses): + import google.cloud.storage._http + from google.cloud.exceptions import NotFound + + mock_conn = mock.create_autospec(google.cloud.storage._http.Connection) + mock_conn.user_agent = "testing 1.2.3" + mock_conn.api_request.side_effect = list(responses) + [NotFound("miss")] + return mock_conn + + def _make_response(status=http_client.OK, content=b"", headers={}): response = requests.Response() response.status_code = status @@ -645,7 +655,7 @@ def test_list_blobs(self): credentials = _make_credentials() client = self._make_one(project="PROJECT", credentials=credentials) - connection = _Connection({"items": []}) + connection = _make_connection({"items": []}) with mock.patch( 'google.cloud.storage.client.Client._connection', @@ -658,10 +668,11 @@ def test_list_blobs(self): blobs = list(iterator) self.assertEqual(blobs, []) - kw, = connection._requested - self.assertEqual(kw["method"], "GET") - self.assertEqual(kw["path"], "/b/%s/o" % BUCKET_NAME) - self.assertEqual(kw["query_params"], {"projection": "noAcl"}) + connection.api_request.assert_called_once_with( + method="GET", + path="/b/%s/o" % BUCKET_NAME, + query_params={"projection": "noAcl"} + ) def test_list_blobs_w_all_arguments_and_user_project(self): from google.cloud.storage.bucket import Bucket @@ -687,7 +698,7 @@ def test_list_blobs_w_all_arguments_and_user_project(self): credentials = _make_credentials() client = self._make_one(project=USER_PROJECT, credentials=credentials) - connection = _Connection({"items": []}) + connection = _make_connection({"items": []}) with mock.patch( 'google.cloud.storage.client.Client._connection', @@ -709,10 +720,11 @@ def test_list_blobs_w_all_arguments_and_user_project(self): blobs = list(iterator) self.assertEqual(blobs, []) - kw, = connection._requested - self.assertEqual(kw["method"], "GET") - self.assertEqual(kw["path"], "/b/%s/o" % BUCKET_NAME) - self.assertEqual(kw["query_params"], EXPECTED) + connection.api_request.assert_called_once_with( + method="GET", + path="/b/%s/o" % BUCKET_NAME, + query_params=EXPECTED + ) def test_list_buckets_wo_project(self): CREDENTIALS = _make_credentials() @@ -896,39 +908,3 @@ def dummy_response(): self.assertEqual(page.remaining, 0) self.assertIsInstance(bucket, Bucket) self.assertEqual(bucket.name, blob_name) - - -class _Connection(object): - _delete_bucket = False - - def __init__(self, *responses): - self._responses = responses - self._requested = [] - self._deleted_buckets = [] - self.credentials = None - - @staticmethod - def _is_bucket_path(path): - # Now just ensure the path only has /b/ and one more segment. - return path.startswith("/b/") and path.count("/") == 2 - - def api_request(self, **kw): - from google.cloud.exceptions import NotFound - - self._requested.append(kw) - - method = kw.get("method") - path = kw.get("path", "") - if method == "DELETE" and self._is_bucket_path(path): - self._deleted_buckets.append(kw) - if self._delete_bucket: - return - else: - raise NotFound("miss") - - try: - response, self._responses = self._responses[0], self._responses[1:] - except IndexError: - raise NotFound("miss") - else: - return response From 42b1a40bc1c14bc879d3a54cc76dcbca1f438b80 Mon Sep 17 00:00:00 2001 From: Gurov Ilya Date: Fri, 28 Jun 2019 11:04:00 +0300 Subject: [PATCH 8/8] Nit on docing Add backticks for sphinx --- 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 81f03e190539..79755897529c 100644 --- a/storage/google/cloud/storage/bucket.py +++ b/storage/google/cloud/storage/bucket.py @@ -722,7 +722,7 @@ def list_blobs( """Return an iterator used to find blobs in the bucket. .. note:: - Direct use of this method is deprecated. Use Client.list_blobs instead. + Direct use of this method is deprecated. Use ``Client.list_blobs`` instead. If :attr:`user_project` is set, bills the API request to that project.