storage: cannot set object properties

[pdknsk]

Or at least I haven't figured out how. This is how the documentation suggests it may work.

>>> from gcloud import storage
>>> bucket = storage.get_bucket('bucket', 'id')
>>> key = storage.key.Key(bucket=bucket, name='dir/file.webp', 
...                       properties={'contentType': 'image/webp'})
>>> key.upload_from_filename('dir/file.webp')

Success?

>>> key.properties['contentType']
'image/webp'
>>> key.content_type
'image/webp'

It's a lie unfortunately. (Also verified with gsutil).

>>> webp = bucket.get_key('dir/file.webp')
>>> webp.properties['contentType']
u'application/unknown'
>>> webp.content_type
u'application/unknown'

[pdknsk]

Just an additional minor remark. gsutil prefers application/octet-stream to application/unknown.

[dhermes]

@pdknsk Thanks for the feedback, we'll look into it. (Also thanks for hunting down a more sensible default.)

[tseaver]

Note that upload_from_filename overwrites any previously-set contentType (via upload_from_file). So, the fix would be to call self._reload_properties() at the end of upload_from_file.

[pdknsk]

I applied the fix and the result is this.

>>> key.content_type
'image/webp'
>>> key.upload_from_filename('dir/file.webp')
>>> key.content_type
u'application/unknown'

Which is correct in a way, because that's how the object is stored online, even if incorrectly so.

I tried Cache-Control (or cacheControl rather) and it doesn't work either.

[pdknsk]

I've figured out that in order to send metadata with the request it has to be set on the request body before upload.ConfigureRequest is called. This works for Content-Type and Cache-Control in this case.

--- a/gcloud/storage/blob.py
+++ b/gcloud/storage/blob.py
@@ -296,7 +296,7 @@ class Blob(_PropertyMixin):
         }

         upload = transfer.Upload(file_obj,
-                                 content_type or 'application/unknown',
+                                 content_type or self.properties.get('contentType', 'application/unknown'),
                                  total_bytes, auto_transfer=False,
                                  chunksize=self.CHUNK_SIZE)

@@ -311,6 +311,11 @@ class Blob(_PropertyMixin):
         # Use apitools 'Upload' facility.
         request = http_wrapper.Request(upload_url, 'POST', headers)

+        if self.properties:
+            import json
+            headers['content-type'] = 'application/json'
+            request.body = json.dumps(self.properties)
+
         upload.ConfigureRequest(upload_config, request, url_builder)
         query_params = url_builder.query_params
         request.url = conn.build_api_url(path=self.bucket.path + '/o',

[dhermes]

So the core issue here is that Blob.upload_from_filename uses mimetypes rather than the given content_type. I tested with a real webp file and mimetypes doesn't know what to think.

You could "get around this" by using upload_from_file directly.

[dhermes]

@tseaver I'm going to send a fix and we should discuss the "right approach" in the PR.

[pdknsk]

I haven't tried but from what I can tell 83c0fc1 only fixes Content-Type but not the other properties, called Optional Properties in the docs. The underlying library nicely does all the work on this when it detects that request.body is set.

[dhermes]

@pdknsk Yeah, per my comments in #629 I think we're better off having a clear Blob.create() and Blob.update(). Still trying to figure out what the best API surface is.

[pdknsk]

The only difference I noticed in setting the metadata separately afterwards is that it increases metageneration version. So you wouldn't be able to make an object with metageneration 1. I have no clue if anyone would need it — I very much doubt it.

And another remark on this bug. The gsutil documentation has an interesting comment stating that if Content-Type is not set, it will be set server-side.

For example, at the time of writing this, the filename extension ”.webp” was recognized by the server-side content detection system, but not by gsutil.

I haven't been able to get this to work, using gsutil at least.

[dhermes]

Thanks for digging more. I filed #632 and hopefully (if we move forward on it) something like Blob.create() will handle this correctly.

[dhermes]

@pdknsk So I merged the fix, but you indicated other things are not addressed.

Essentially upload doesn't send properties is the issue?

It seems instead of blob._reload_properties() (as @tseaver said) we would want to call

blob._patch_properties(blob._properties)

at the end of upload. WDYT? Shall we open a new bug (not specific to content type)?

Or instead do you think the plan put forth in #632 is sufficient?

[pdknsk]

I tried and it works well.

It does however also set metageneration version to 2. As I already said, I have no clue if anyone needs metageneration set to 1.

Another potential problem is that between both separate requests, the object is very briefly online with incorrect properties, I think. This could theoretically bite anyone who uses permissive acl by default on the bucket, and sets a restrictive acl per object. There is also the freak chance that your connection fails just after the first request, which makes it not so brief.

[dhermes]

Yeah it needs a lot of work, thanks for verifying. Hopefully #632 leads to something useable.