Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storage: updating Connection docstring; turning make_request private. #604

Merged

Conversation

dhermes
Copy link
Contributor

@dhermes dhermes commented Feb 10, 2015

Making it clear in the docstring that the Connection itself only handles storage.bucket... API methods and that the other classes handle the other 30 (give or take) methods of the API.

@tseaver This documents our decision from earlier today (about the responsibilities of Connection).

Also note

  • We don't directly support storage.buckets.update anywhere (but Connection.api_request would allow it)
  • We don't support storage.buckets.patch on Connection. Only via Bucket (through _PropertyMixin._patch_properties()).
  • Both Bucket._reload_properties() (via _PropertyMixin) and Connection.get_bucket implement storage.buckets.get.

For your reference: discovery document.

Making it clear in the docstring that the Connection itself only handles
"bucket" queries and that the other classes handle the other 30 (give or
take) methods of the API.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 10, 2015
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 58edd2e on dhermes:update-storage-docstring-connection into a6317d1 on GoogleCloudPlatform:master.

@tseaver
Copy link
Contributor

tseaver commented Feb 10, 2015

LGTM

dhermes added a commit that referenced this pull request Feb 10, 2015
Storage: updating Connection docstring; turning make_request private.
@dhermes dhermes merged commit b395360 into googleapis:master Feb 10, 2015
@dhermes dhermes deleted the update-storage-docstring-connection branch February 10, 2015 04:02
@dhermes
Copy link
Contributor Author

dhermes commented Feb 10, 2015

@tseaver WDYT of missing support / overlapping support of storage.buckets API methods?

@tseaver
Copy link
Contributor

tseaver commented Feb 11, 2015

I think we should remove the single-bucket-centric connection methods, and surface such operations only on Bucket methods.

  • bucket = connection.get_bucket(bucket_name) -> bucket = Bucket(bucket_name, connection=connection)
  • bucket = connection.create_bucket(bucket_name) -> bucket = Bucket(bucket_name, connection=connection, create=True)
  • Connection.delete_bucket(bucket_name) -> Bucket(bucket_name, connection=connection).delete()

@dhermes
Copy link
Contributor Author

dhermes commented Feb 12, 2015

I like it! In the implicit case, WDYT of:

>>> b = Bucket(bucket_name)
>>> b.exists()
False
>>> b.create()
>>> b.exists()
True

@dhermes
Copy link
Contributor Author

dhermes commented Feb 12, 2015

In line with this would be putting most of the storage.objects... methods on Blob.

This leaves us storage.objects.list and storage.buckets.list in a somewhat strange place but (again, in an implicit CNXN future) we could say

storage.set_default_connection()

buckets = storage.get_all_buckets()

bucket = buckets.next()  # Iterator
for blob in bucket.get_all_blobs():
    do_something(blob)

or even in the more extreme scenario that there is a default bucket

storage.set_defaults()

for blob in storage.get_all_blobs():
    do_something(blob)

@tseaver
Copy link
Contributor

tseaver commented Feb 12, 2015

I like b.create() and b.exists().

I'm not sure about calling buckets.next() manually: how do I know the first one is the one I want?

ISTM that storage.set_defaults() should probably return the default bucket, if there is one defined, or None.

vchudnov-g pushed a commit that referenced this pull request Sep 20, 2023
* feat: Add support for python 3.11

chore: Update gapic-generator-python to v1.8.0
PiperOrigin-RevId: 500768693

Source-Link: googleapis/googleapis@190b612

Source-Link: googleapis/googleapis-gen@7bf29a4
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiN2JmMjlhNDE0YjllY2FjMzE3MGYwYjY1YmRjMmE5NTcwNWMwZWYxYSJ9

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants