Proposal: move API-methods to client #38

tswast · 2019-04-22T16:39:28Z

The Cloud Storage Python client was one of the first google-cloud-python clients, intended to be more reliable and more Pythonic than the google-api-client that it replaced. It is also a 100% hand-written client. Because it predates the auto-generated google-cloud-python clients, the Cloud Storage Python client has inconsistencies with other google-cloud-python clients.

This design proposal is to bring the Cloud Storage client further into alignment with other google-cloud-python clients and make the eventual inclusion of an auto-generated gRPC transport layer less disruptive to users of the Cloud Storage client.

Design document:

https://docs.google.com/document/d/1A4FIxThZK_enK7OV9ChY54IPrlpbdHwuhvVEu6iTHxM/edit?usp=sharing

Please share your feedback on the specifics of this as comments in the design document.

CC @crwilcox @frankyn @lbristol88

Update: according to comment, we need to update samples for moved methods before deprecating the old ones.

…to 'Client' (#8604) towards #7762

frankyn · 2020-03-23T05:36:04Z

@IlyaFaer I believe this is complete. I'm closing FR.

If there's something that I missed please reopen.

IlyaFaer · 2020-03-23T08:48:42Z

@frankyn, I've delegated the last part to @paul1319
@paul1319, do you have something to push for this issue?

frankyn · 2020-03-23T17:58:43Z

Reopening in case there's something pending.

frankyn · 2020-03-31T16:55:56Z

@IlyaFaer haven't heard back for a bit. What's remaining?

paul1319 · 2020-03-31T16:59:01Z

@frankyn as I see it, Blob class Refactor and Deprecate part.

Fixes #38

tseaver · 2021-05-04T18:17:50Z

ISTM that this effort is not complete: e.g., the client does not have a get_blob method, nor any of the notification-related methods for buckets.

This partial refactoring complicates the tests a lot -- if it were complete, then bucket / blob unit tests would just use mocked clients and assert that the corresponding method is called correctly, rather than creating mock connections (and real clients, see #416) and grubbing around three layers down.

Basically, any non-client method using the client's _connection to make API requests should instead be calling a method of the client:

$ git grep api_request google/cloud/storage/{_helpers,blob,bucket,hmac_key,notification}.py
google/cloud/storage/_helpers.py:        api_response = client._connection.api_request(
google/cloud/storage/_helpers.py:        api_response = client._connection.api_request(
google/cloud/storage/_helpers.py:        api_response = client._connection.api_request(
google/cloud/storage/blob.py:            client._connection.api_request(
google/cloud/storage/blob.py:        info = client._connection.api_request(
google/cloud/storage/blob.py:        info = client._connection.api_request(
google/cloud/storage/blob.py:        resp = client._connection.api_request(
google/cloud/storage/blob.py:        api_response = client._connection.api_request(
google/cloud/storage/blob.py:        api_response = client._connection.api_request(
google/cloud/storage/bucket.py:            client._connection.api_request(
google/cloud/storage/bucket.py:        api_request = functools.partial(
google/cloud/storage/bucket.py:            client._connection.api_request, timeout=timeout, retry=retry
google/cloud/storage/bucket.py:            api_request=api_request,
google/cloud/storage/bucket.py:        client._connection.api_request(
google/cloud/storage/bucket.py:        client._connection.api_request(
google/cloud/storage/bucket.py:        copy_result = client._connection.api_request(
google/cloud/storage/bucket.py:        info = client._connection.api_request(
google/cloud/storage/bucket.py:        info = client._connection.api_request(
google/cloud/storage/bucket.py:        resp = client._connection.api_request(
google/cloud/storage/bucket.py:        api_response = client._connection.api_request(
google/cloud/storage/hmac_key.py:            self._client._connection.api_request(
google/cloud/storage/hmac_key.py:        self._properties = self._client._connection.api_request(
google/cloud/storage/hmac_key.py:        self._properties = self._client._connection.api_request(
google/cloud/storage/hmac_key.py:        self._client._connection.api_request(
google/cloud/storage/notification.py:        self._properties = client._connection.api_request(
google/cloud/storage/notification.py:            client._connection.api_request(
google/cloud/storage/notification.py:        response = client._connection.api_request(
google/cloud/storage/notification.py:        client._connection.api_request(

Toward #38.

Also, adjust tests for 'Bucket.list_blobs' not to depend on anything but calling 'Client.list_blobs'. Toward #38

Toward #38

Also, provide explicit coverage for 'client._item_to_bucket' and 'client._item_to_hmack_key_metadata' helpers. Toward #38

Also, provide explicit coverage for 'bucket._item_to_notification'. Toward #38

Toward #38.

Also, adjust tests for 'Bucket.list_blobs' not to depend on anything but calling 'Client.list_blobs'. Toward #38

Toward #38

Also, provide explicit coverage for 'client._item_to_bucket' and 'client._item_to_hmack_key_metadata' helpers. Toward #38

Also, provide explicit coverage for 'bucket._item_to_notification'. Toward #38

* Adjust tests for 'Bucket.list_blobs' not to depend on anything but calling 'Client.list_blobs'. * Provide explicit coverage for 'client._item_to_bucket' and 'client._item_to_hmack_key_metadata' helpers. * Provide explicit coverage for 'bucket._item_to_notification'. Toward #38

Fixes googleapis#38

Use an explicit helper client method for `GET` requests, rather than manipulating client's private `_connection.api_request`. As a benefit, tests get *way* clearer. Toward googleapis#38 ~~Based on top of the branch from googleapis#430. I will rebase when that PR merges.~~

Add retry support to 'ACL.save', 'ACL.save_predefined', and 'ACL.clear'. Add 'timeout' and 'retry' support to 'Blob.make_private'. Toward googleapis#38.

Toward googleapis#38.

Also, forward 'retry' through when deleting blobs during 'Bucket.delete'. Toward googleapis#38.

Toward googleapis#38.

* Adjust tests for 'Bucket.list_blobs' not to depend on anything but calling 'Client.list_blobs'. * Provide explicit coverage for 'client._item_to_bucket' and 'client._item_to_hmack_key_metadata' helpers. * Provide explicit coverage for 'bucket._item_to_notification'. Toward googleapis#38

Fixes googleapis#38

Use an explicit helper client method for `GET` requests, rather than manipulating client's private `_connection.api_request`. As a benefit, tests get *way* clearer. Toward googleapis#38 ~~Based on top of the branch from googleapis#430. I will rebase when that PR merges.~~

Add retry support to 'ACL.save', 'ACL.save_predefined', and 'ACL.clear'. Add 'timeout' and 'retry' support to 'Blob.make_private'. Toward googleapis#38.

Toward googleapis#38.

Also, forward 'retry' through when deleting blobs during 'Bucket.delete'. Toward googleapis#38.

Toward googleapis#38.

* Adjust tests for 'Bucket.list_blobs' not to depend on anything but calling 'Client.list_blobs'. * Provide explicit coverage for 'client._item_to_bucket' and 'client._item_to_hmack_key_metadata' helpers. * Provide explicit coverage for 'bucket._item_to_notification'. Toward googleapis#38

cojenco · 2022-08-10T21:36:38Z

I believe the refactor is complete through #431 #436 #441 #442 #443 #446

tswast changed the title ~~Storage: redesign proposal~~ Storage: client redesign proposal Apr 22, 2019

IlyaFaer self-assigned this Aug 2, 2019

IlyaFaer referenced this issue in googleapis/google-cloud-python Nov 8, 2019

refactor(storage): move 'create_bucket' implementation from 'Bucket' …

332c8ff

…to 'Client' (#8604) towards #7762

crwilcox transferred this issue from googleapis/google-cloud-python Jan 31, 2020

product-auto-label bot added the api: storage Issues related to the googleapis/python-storage API. label Jan 31, 2020

yoshi-automation added 🚨 This issue needs some love. triage me I really want to be triaged. labels Feb 3, 2020

busunkim96 removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Feb 4, 2020

yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Feb 4, 2020

frankyn added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Feb 4, 2020

IlyaFaer mentioned this issue Feb 10, 2020

refactor(storage): deprecate Bucket.create() and requester_pays #49

Merged

frankyn closed this as completed Mar 23, 2020

frankyn reopened this Mar 23, 2020

paul1319 mentioned this issue Jun 18, 2020

refactor(storage): move 'list_blobs' implementation from 'Bucket' and 'download_blob_to_file' implementation from 'Blob' to Client #184

Closed

tseaver changed the title ~~Storage: client redesign proposal~~ Proposal: move API-methods to client Aug 17, 2020

HemangChothani mentioned this issue Nov 12, 2020

refactor: move methods to client class #311

Merged

gcf-merge-on-green bot closed this as completed in #311 Dec 11, 2020

gcf-merge-on-green bot pushed a commit that referenced this issue Dec 11, 2020

refactor: move methods to client class (#311)

5b4568e

Fixes #38

tseaver reopened this May 4, 2021

tseaver unassigned IlyaFaer May 4, 2021

tseaver mentioned this issue Jun 8, 2021

fix: apply idempotency policies for ACLs #458

Merged

tseaver added a commit that referenced this issue Jun 8, 2021

refactor: add 'Client._list_resource' method

f30af4d

Toward #38.

tseaver added a commit that referenced this issue Jun 8, 2021

refactor: use 'Client._list_resource' in 'Client.list_blobs'

15fb51a

Also, adjust tests for 'Bucket.list_blobs' not to depend on anything but calling 'Client.list_blobs'. Toward #38

tseaver added a commit that referenced this issue Jun 8, 2021

refactor: use 'Client._list_resource' in 'Client.list_buckets'

8bdc81b

Toward #38

tseaver added a commit that referenced this issue Jun 8, 2021

refactor: use 'Client._list_resource' in 'Client.list_hmac_keys'

3688363

Also, provide explicit coverage for 'client._item_to_bucket' and 'client._item_to_hmack_key_metadata' helpers. Toward #38

tseaver added a commit that referenced this issue Jun 8, 2021

refactor: use 'Client._list_resource' in 'Bucket.list_notificatoins'

5024d1f

Also, provide explicit coverage for 'bucket._item_to_notification'. Toward #38

tseaver added a commit that referenced this issue Jun 8, 2021

refactor: add 'Client._list_resource' method

4d0cb8b

Toward #38.

tseaver added a commit that referenced this issue Jun 8, 2021

refactor: use 'Client._list_resource' in 'Client.list_blobs'

0352fdc

Also, adjust tests for 'Bucket.list_blobs' not to depend on anything but calling 'Client.list_blobs'. Toward #38

tseaver added a commit that referenced this issue Jun 8, 2021

refactor: use 'Client._list_resource' in 'Client.list_buckets'

211d22d

Toward #38

tseaver added a commit that referenced this issue Jun 8, 2021

refactor: use 'Client._list_resource' in 'Client.list_hmac_keys'

8c6c4e8

Also, provide explicit coverage for 'client._item_to_bucket' and 'client._item_to_hmack_key_metadata' helpers. Toward #38

tseaver added a commit that referenced this issue Jun 8, 2021

refactor: use 'Client._list_resource' in 'Bucket.list_notificatoins'

68b762e

Also, provide explicit coverage for 'bucket._item_to_notification'. Toward #38

tseaver removed their assignment Sep 30, 2021

cojenco pushed a commit to cojenco/python-storage that referenced this issue Oct 13, 2021

refactor: move methods to client class (googleapis#311)

2519698

Fixes googleapis#38

cojenco pushed a commit to cojenco/python-storage that referenced this issue Oct 13, 2021

refactor: add / use 'Client._put_resource' method (googleapis#441)

b9d4058

Toward googleapis#38.

cojenco pushed a commit to cojenco/python-storage that referenced this issue Oct 13, 2021

refactor: add / use 'Client._delete_resource' method (googleapis#442)

c5d2fcb

Also, forward 'retry' through when deleting blobs during 'Bucket.delete'. Toward googleapis#38.

cojenco pushed a commit to cojenco/python-storage that referenced this issue Oct 13, 2021

refactor: add / use 'Client._post_resource' method (googleapis#443)

c319578

Toward googleapis#38.

cojenco pushed a commit to cojenco/python-storage that referenced this issue Oct 13, 2021

refactor: move methods to client class (googleapis#311)

e0df981

Fixes googleapis#38

cojenco pushed a commit to cojenco/python-storage that referenced this issue Oct 13, 2021

refactor: add / use 'Client._put_resource' method (googleapis#441)

ac168b3

Toward googleapis#38.

cojenco pushed a commit to cojenco/python-storage that referenced this issue Oct 13, 2021

refactor: add / use 'Client._delete_resource' method (googleapis#442)

d5880e0

Also, forward 'retry' through when deleting blobs during 'Bucket.delete'. Toward googleapis#38.

cojenco pushed a commit to cojenco/python-storage that referenced this issue Oct 13, 2021

refactor: add / use 'Client._post_resource' method (googleapis#443)

2c2cd1b

Toward googleapis#38.

cojenco closed this as completed Aug 10, 2022

frankyn assigned cojenco Aug 11, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Proposal: move API-methods to client #38

Proposal: move API-methods to client #38

tswast commented Apr 22, 2019

frankyn commented Mar 23, 2020

IlyaFaer commented Mar 23, 2020

frankyn commented Mar 23, 2020

frankyn commented Mar 31, 2020

paul1319 commented Mar 31, 2020 •

edited

Loading

tseaver commented May 4, 2021 •

edited

Loading

cojenco commented Aug 10, 2022

Proposal: move API-methods to client #38

Proposal: move API-methods to client #38

Comments

tswast commented Apr 22, 2019

frankyn commented Mar 23, 2020

IlyaFaer commented Mar 23, 2020

frankyn commented Mar 23, 2020

frankyn commented Mar 31, 2020

paul1319 commented Mar 31, 2020 • edited Loading

tseaver commented May 4, 2021 • edited Loading

cojenco commented Aug 10, 2022

paul1319 commented Mar 31, 2020 •

edited

Loading

tseaver commented May 4, 2021 •

edited

Loading