Storage: add 'Client.list_blobs(bucket_or_name)'. #8375

IlyaFaer · 2019-06-17T09:03:53Z

Made item from redesign doc: "New method: client.list_blobs(bucket_or_name), which functions the same as Bucket.list_blobs()"
https://github.com/googleapis/google-cloud-python/issues/7762

…obs_defaults

IlyaFaer · 2019-06-17T09:05:26Z

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):


Made a helper - for now it's used in 3 places

Nit on naming: how about _bucket_arg_to_bucket to align with the similar helper in BigQuery?

google-cloud-python/bigquery/google/cloud/bigquery/table.py

Line 1771 in 0e231ca

def _table_arg_to_table(value, default_project=None):

Also, this might be well suited for the bucket.py module, similar to BigQuery, as I expect the client.py file will expand and the bucket.py file will shrink as we move implementation over to get ready for deprecation.

No problem, renamed

IlyaFaer · 2019-06-17T09:06:24Z

storage/tests/unit/test_bucket.py

-        kw, = connection._requested
-        self.assertEqual(kw["method"], "GET")
-        self.assertEqual(kw["path"], "/b/%s/o" % NAME)
-        self.assertEqual(kw["query_params"], {"projection": "noAcl"})


This is full copy of another test named test_list_blobs_defaults

IlyaFaer · 2019-06-17T09:07:58Z

storage/tests/unit/test_client.py

@@ -821,3 +896,39 @@ def dummy_response():
        self.assertEqual(page.remaining, 0)
        self.assertIsInstance(bucket, Bucket)
        self.assertEqual(bucket.name, blob_name)
+
+
+class _Connection(object):


Analogy to class from bucket tests

I'd prefer that we use mocks rather than add this class.

See:

google-cloud-python/bigquery/tests/unit/helpers.py

Lines 16 to 24 in 0e231ca

def make_connection(*responses):

import google.cloud.bigquery._http

import mock

from google.cloud.exceptions import NotFound

mock_conn = mock.create_autospec(google.cloud.bigquery._http.Connection)

mock_conn.user_agent = "testing 1.2.3"

mock_conn.api_request.side_effect = list(responses) + [NotFound("miss")]

return mock_conn

and how it is used:

google-cloud-python/bigquery/tests/unit/test_client.py

Lines 169 to 173 in 0e231ca

conn.api_request.assert_called_once_with(

method="GET",

path="/projects/other-project/queries/nothere",

query_params={"maxResults": 0, "timeoutMs": 500, "location": self.LOCATION},

)

Redone with similar mock

IlyaFaer · 2019-06-17T10:59:24Z

storage/google/cloud/storage/bucket.py

+            "Bucket.list_blobs method is deprecated. Use Client.list_blobs instead.",
+            PendingDeprecationWarning,
+            stacklevel=2,
+        )


Not sure about this. It will warn if you call Client.list_blobs too

Let's wait to mark these as deprecated until we move all the actual implementation out of the Blob and Bucket classes into the Client class.

tswast · 2019-06-21T23:28:27Z

storage/google/cloud/storage/bucket.py

+            "Bucket.list_blobs method is deprecated. Use Client.list_blobs instead.",
+            PendingDeprecationWarning,
+            stacklevel=2,
+        )


Let's wait to mark these as deprecated until we move all the actual implementation out of the Blob and Bucket classes into the Client class.

tswast · 2019-06-21T23:33:39Z

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):


Nit on naming: how about _bucket_arg_to_bucket to align with the similar helper in BigQuery?

google-cloud-python/bigquery/google/cloud/bigquery/table.py

Line 1771 in 0e231ca

def _table_arg_to_table(value, default_project=None):

Also, this might be well suited for the bucket.py module, similar to BigQuery, as I expect the client.py file will expand and the bucket.py file will shrink as we move implementation over to get ready for deprecation.

tswast · 2019-06-21T23:34:23Z

storage/google/cloud/storage/client.py

+        ]):
+            The bucket resource to pass or name to create.
+
+        :type max_results: int


Use Google-style docstrings. https://sphinxcontrib-napoleon.readthedocs.io/en/latest/example_google.html#example-google

Okey, it's done

tswast · 2019-06-21T23:37:46Z

storage/tests/unit/test_client.py

@@ -821,3 +896,39 @@ def dummy_response():
        self.assertEqual(page.remaining, 0)
        self.assertIsInstance(bucket, Bucket)
        self.assertEqual(bucket.name, blob_name)
+
+
+class _Connection(object):


I'd prefer that we use mocks rather than add this class.

See:

google-cloud-python/bigquery/tests/unit/helpers.py

Lines 16 to 24 in 0e231ca

def make_connection(*responses):

import google.cloud.bigquery._http

import mock

from google.cloud.exceptions import NotFound

mock_conn = mock.create_autospec(google.cloud.bigquery._http.Connection)

mock_conn.user_agent = "testing 1.2.3"

mock_conn.api_request.side_effect = list(responses) + [NotFound("miss")]

return mock_conn

and how it is used:

google-cloud-python/bigquery/tests/unit/test_client.py

Lines 169 to 173 in 0e231ca

conn.api_request.assert_called_once_with(

method="GET",

path="/projects/other-project/queries/nothere",

query_params={"maxResults": 0, "timeoutMs": 500, "location": self.LOCATION},

)

tswast

Thanks! One nit, but otherwise LGTM.

Thanks for your patience.

tswast · 2019-06-27T19:31:30Z

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.


Nit. Add backticks for sphinx to render as code font.

Use ``Client.list_blobs`` instead.

Add backticks for sphinx

IlyaFaer added 5 commits June 14, 2019 16:22

Union create-or-use bucket snippets into one helper

ad54362

Deleted test_list_blobs - looks like it's a full copy of test_list_bl…

1835337

…obs_defaults

Add new method client.list_blobs that's the same as bucket.list_blobs()

e038d19

Merge branch 'master' into storage_client_redesign_7762

107a02b

Redact comment and delete unittest runner

e120a23

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jun 17, 2019

IlyaFaer commented Jun 17, 2019

View reviewed changes

IlyaFaer marked this pull request as ready for review June 17, 2019 09:08

IlyaFaer requested review from crwilcox, frankyn and tseaver as code owners June 17, 2019 09:08

IlyaFaer added 2 commits June 17, 2019 12:18

Add deprecation for bucket.list_blobs

8175788

Changed warn to comment

7e20131

IlyaFaer commented Jun 17, 2019

View reviewed changes

frankyn requested a review from tswast June 17, 2019 16:37

tseaver changed the title ~~Storage client redesign 7762~~ Storage: add 'Client.list_blobs(bucket_or_name)'. Jun 19, 2019

tseaver added the api: storage Issues related to the Cloud Storage API. label Jun 19, 2019

tswast reviewed Jun 21, 2019

View reviewed changes

Merge branch 'master' into storage_client_redesign_7762

04fc185

yoshi-automation added the 🚨 This issue needs some love. label Jun 24, 2019

Bring method docs to Google-style, use mock connection instead of class

f083156

tswast approved these changes Jun 27, 2019

View reviewed changes

Nit on docing

42b1a40

Add backticks for sphinx

tswast merged commit c737b74 into googleapis:master Jun 28, 2019

IlyaFaer deleted the storage_client_redesign_7762 branch July 2, 2019 11:32

IlyaFaer mentioned this pull request Jul 3, 2019

[GCP-7762] Storage: client redesign proposal MaxxleLLC/google-cloud-python#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Storage: add 'Client.list_blobs(bucket_or_name)'. #8375

Storage: add 'Client.list_blobs(bucket_or_name)'. #8375

IlyaFaer commented Jun 17, 2019 •

edited

Loading

IlyaFaer Jun 17, 2019

tswast Jun 21, 2019

IlyaFaer Jun 24, 2019

IlyaFaer Jun 17, 2019

IlyaFaer Jun 17, 2019

tswast Jun 21, 2019

IlyaFaer Jun 24, 2019

IlyaFaer Jun 17, 2019

tswast Jun 21, 2019

tswast Jun 21, 2019

tswast Jun 21, 2019

tswast Jun 21, 2019

IlyaFaer Jun 24, 2019

tswast Jun 21, 2019

tswast left a comment •

edited

Loading

tswast Jun 27, 2019

	def make_connection(*responses):
	import google.cloud.bigquery._http
	import mock
	from google.cloud.exceptions import NotFound

	mock_conn = mock.create_autospec(google.cloud.bigquery._http.Connection)
	mock_conn.user_agent = "testing 1.2.3"
	mock_conn.api_request.side_effect = list(responses) + [NotFound("miss")]
	return mock_conn

	conn.api_request.assert_called_once_with(
	method="GET",
	path="/projects/other-project/queries/nothere",
	query_params={"maxResults": 0, "timeoutMs": 500, "location": self.LOCATION},
	)

Storage: add 'Client.list_blobs(bucket_or_name)'. #8375

Storage: add 'Client.list_blobs(bucket_or_name)'. #8375

Conversation

IlyaFaer commented Jun 17, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tswast left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IlyaFaer commented Jun 17, 2019 •

edited

Loading

tswast left a comment •

edited

Loading