support for predefined ACL to be set directly on the upload request

[pdknsk]

This is a follow-up to #651.

This request may be a bit too early, as it will only really matter when #1185 is fixed. Right now with blob.acl.save_predefined the object ACL can be set to a predefined value separately, like the metadata can be with blob.patch. The idea is to set everything with the upload request, to not have separate request.

[pdknsk]

I've made a simple patch for common use cases, on top of another patch. It does not check the name of the predefined ACL, nor does it translate from XML to JSON names.

--- a/blob.py
+++ b/blob.py
@@ -407,7 +407,8 @@
 
     def _create_upload(
             self, client, file_obj=None, size=None, content_type=None,
-            chunk_size=None, strategy=None, extra_headers=None):
+            chunk_size=None, strategy=None, extra_headers=None,
+            predefined_acl=None):
         """Helper for upload methods.
 
         Creates a :class:`google.cloud.core.streaming.Upload` object to handle
@@ -482,6 +483,7 @@
             upload.strategy = RESUMABLE_UPLOAD
 
         url_builder = _UrlBuilder(
+            predefined_acl=predefined_acl,
             bucket_name=self.bucket.name,
             object_name=self.name)
         upload_config = _UploadConfig()
@@ -497,6 +499,9 @@
 
         if self._properties:
             headers['content-type'] = 'application/json'
+            # 409 Cannot provide both a predefinedAcl and access controls
+            if predefined_acl and 'acl' in self._properties:
+                del self._properties['acl']
             request.body = json.dumps(self._properties)
 
         upload.configure_request(upload_config, request, url_builder)
@@ -525,7 +530,8 @@
                                  error_info=request.url)
 
     def upload_from_file(self, file_obj, rewind=False, size=None,
-                         content_type=None, num_retries=6, client=None):
+                         content_type=None, num_retries=6, client=None,
+                         predefined_acl=None):
         """Upload the contents of this blob from a file-like object.
 
         The content type of the upload will either be
@@ -621,7 +627,7 @@
         upload, request, _ = self._create_upload(
             client, file_obj=file_obj, size=total_bytes,
             content_type=content_type, chunk_size=chunk_size,
-            strategy=strategy)
+            strategy=strategy, predefined_acl=predefined_acl)
 
         if upload.strategy == RESUMABLE_UPLOAD:
             http_response = upload.stream_file(use_chunks=True)
@@ -637,7 +643,8 @@
             response_content = response_content.decode('utf-8')
         self._set_properties(json.loads(response_content))
 
-    def upload_from_filename(self, filename, content_type=None, client=None):
+    def upload_from_filename(self, filename, content_type=None, client=None,
+                             predefined_acl=None):
         """Upload this blob's contents from the content of a named file.
 
         The content type of the upload will either be
@@ -675,7 +682,8 @@
             self.upload_from_file(
                 file_obj, content_type=content_type, client=client)
 
-    def upload_from_string(self, data, content_type='text/plain', client=None):
+    def upload_from_string(self, data, content_type='text/plain', client=None,
+                           predefined_acl=None):
         """Upload contents of this blob from the provided string.
 
         .. note::
@@ -708,7 +716,8 @@
         string_buffer.write(data)
         self.upload_from_file(
             file_obj=string_buffer, rewind=True, size=len(data),
-            content_type=content_type, client=client)
+            content_type=content_type, client=client,
+            predefined_acl=predefined_acl)
 
     def upload_from_bytes(self, data, content_type=None, client=None):
 
@@ -1211,8 +1220,10 @@
 
 class _UrlBuilder(object):
     """Faux builder FBO apitools' 'configure_request'"""
-    def __init__(self, bucket_name, object_name):
+    def __init__(self, bucket_name, object_name, predefined_acl):
         self.query_params = {'name': object_name}
+        if predefined_acl:
+            self.query_params['predefinedAcl'] = predefined_acl
         self._bucket_name = bucket_name
         self._relative_path = ''
 

[pdknsk]

--- a/blob.py
+++ b/blob.py
@@ -680,7 +680,8 @@
 
         with open(filename, 'rb') as file_obj:
             self.upload_from_file(
-                file_obj, content_type=content_type, client=client)
+                file_obj, content_type=content_type, client=client,
+                predefined_acl=predefined_acl)
 
     def upload_from_string(self, data, content_type='text/plain', client=None,
                            predefined_acl=None):

[chemelnucfin]

Hello, may I ask if you can make a pull request with this patch?

Or may I submit a PR on your behalf using the code above?

[pdknsk]

Sure, though I think you should perhaps add XML names to JSON names translation.

[chemelnucfin]

Hello, feature requests will now be tracked in the project Feature Requests. I will close this issue now, please feel free to continue to address any issues/concerns here.