Conversation
cojenco
left a comment
There was a problem hiding this comment.
Looking good 🎉 have a few questions
|
Added integration tests. PTAL |
|
PTAL once more; this should be the final feature set I think |
| filename, | ||
| blob, | ||
| content_type=None, | ||
| chunk_size=TM_DEFAULT_CHUNK_SIZE, |
There was a problem hiding this comment.
this is technically MPU part_size? Are using chunk_size for consistency?
There was a problem hiding this comment.
It is the part size. I am using chunk_size for consistency with the similar download method.
| the documentation at https://cloud.google.com/storage/docs/multipart-uploads | ||
| before using this feature. | ||
|
|
||
| The library will attempt to cancel uploads that fail due to an exception. |
danielduhh
left a comment
There was a problem hiding this comment.
Are you planning to add the upload_sharded command name for metrics on in an other PR?
|
@danielduhh Yes, actually that's already ready for review and I'll have it up as soon as this one is reviewed and merged. |
cojenco
left a comment
There was a problem hiding this comment.
LGTM, just have two questions
| return object_metadata | ||
|
|
||
| def _get_upload_arguments(self, client, content_type): | ||
| def _get_upload_arguments(self, client, content_type, filename=None): |
There was a problem hiding this comment.
lgtm just a reminder there are changes in the tm-metrics branch relevant to this private method as well
There was a problem hiding this comment.
Yep, already resolved via merge in a different branch
| headers = {} | ||
| # Handle standard writable metadata | ||
| for key, value in metadata.items(): | ||
| if key in METADATA_HEADER_TRANSLATION: |
There was a problem hiding this comment.
QQ are there any writable metadata fields that don't require translation?
Just wanted to make sure that they are all captured in _headers_from_metadata. If not, this works!
There was a problem hiding this comment.
I believe this is the complete list according to the documentation. Note that custom metadata and some other features like encryption are not supported through this translation dictionary but handled differently.
This adds support for XML MPU multiprocess uploads to Transfer Manager.