STORAGE: Does a connection need to be bound to anything other than a Batch or Iterator?

[dhermes]

Run

$ git grep -n '\.connection' -- gcloud/datastore/ | egrep -v test | egrep -v 'datastore\.connection'
$ git grep -n '\.connection' -- gcloud/storage/ | egrep -v test | egrep -v 'storage\.connection'

to compare uses.

[dhermes]

I'm going to start this work in a series of small changes (I'm guessing there will be a lot of test re-factoring).

Please protest if there are any issues.

UPDATE: After starting this, it seems quite daunting.

After checking blob.py and bucket.py it seems the following methods all use a connection.

From git grep -e '^ def' -- gcloud/storage/blob.py (edited by me):

def generate_signed_url(self, expiration, method='GET'):
def exists(self):
def rename(self, new_name):
def delete(self):
def download_to_file(self, file_obj):
def download_to_filename(self, filename):
def download_as_string(self):
def upload_from_file(self, file_obj, rewind=False, ...
def upload_from_filename(self, filename, ...
def upload_from_string(self, data, ...
def make_public(self):  # Maybe not?

From git grep -e '^ def' -- gcloud/storage/bucket.py (edited by me):

def __iter__(self):
def __contains__(self, blob_name):
def exists(self):
def create(self, project=None):
def get_blob(self, blob_name):
def get_all_blobs(self):
def iterator(self, prefix=None, delimiter=None, max_results=None,
def delete(self, force=False):
def delete_blob(self, blob_name):
def delete_blobs(self, blobs, on_error=None):
def copy_blob(self, blob, destination_bucket, new_name=None):
def upload_file(self, filename, blob_name=None):
def upload_file_object(self, file_obj, blob_name=None):
def make_public(self, recursive=False, future=False):

(Updated March 31, 2015)

[dhermes]

@tseaver We have some methods which make HTTP requests for historical reasons only:

• Bucket.update_cors (Making cors a property instead of having two separate methods. #768)
• Bucket.get_default_object_acl (Removing Bucket.get_default_object_acl. #772)
• Bucket.update_lifecycle (Adding Bucket.lifecycle_rules property to replace methods. #774)
• Bucket.get_logging (Removing HTTP requests from Bucket logging methods. #769)
• Bucket.enable_logging (Removing HTTP requests from Bucket logging methods. #769)
• Bucket.disable_logging (Removing HTTP requests from Bucket logging methods. #769)
• Bucket.configure_website (Removing patch() from Bucket.configure_website. #773)
• Bucket.disable_website (Removing patch() from Bucket.configure_website. #773)
• Bucket.make_public

Also Blob.make_public doesn't really fit the rest of methods, but that makes an HTTP request via the ObjectACL. It seems hairier to unwind this.

[tseaver]

I think we could easily change the metadata-updating methods to drop the call (and require calling patch()). Maybe ACL updates should also require an explict patch or save call?

[dhermes]

@tseaver I agree. Being more declarative was the original goal of #545. I tried to sketch a way (in #761) to make these properties, but some of them are a bit involved (i.e. have multiple values associated).

We could look into using apitools generated clients at some point?

[dhermes]

@tseaver Removing __iter__ and __contains__ is impossible if no Connection is bound to a Bucket. As I mention in #761, using __iter__ is pretty scary for large applications / buckets.