feat: add additional_blob_attributes to upload_many_from_filenames#1162
feat: add additional_blob_attributes to upload_many_from_filenames#1162gcf-merge-on-green[bot] merged 3 commits intomainfrom
Conversation
| blob_name = blob_name_prefix + filename | ||
| blob = bucket.blob(blob_name, **blob_constructor_kwargs) | ||
| for prop, value in additional_blob_attributes.items(): | ||
| setattr(blob, prop, value) |
There was a problem hiding this comment.
Is there a reason not to use blob._patch_property() that is broadly used in the library? Blob property changes are propagated through blob._get_writable_metadata and blob._get_upload_arguments
There was a problem hiding this comment.
I did consider that but decided there was no need to use a private method on the blob class when the typical public interface of attribute assignment would do the same thing.
There was a problem hiding this comment.
I'm curious how the blob properties are actually picked up in blob._changes. I thought we're relying on blob._changes to get writable metadata for the upload arguments
python-storage/google/cloud/storage/blob.py
Lines 1708 to 1713 in c5a983d
There was a problem hiding this comment.
In the _PropertyMixin, all the fields set as a _scalar_property have a custom setter that updates _changes. This solution using setattr will also activate that.
There was a problem hiding this comment.
Ah I see, that makes sense, thanks! One last nit on the naming, maybe consider additional_blob_properties for consistency? Property is used in the official docs and in the lib.
There was a problem hiding this comment.
Haha, that was actually the name in my first revision.
I changed it to "attributes" because it's not only _scalar_property but any attribute that we can set with setattr. Also, since we're using setattr the nomenclature matches.
|
I'm going to add a system test to this before merging, working on it now. |
Fixes #996 🦕