feat: add transfer_manager.upload_chunks_concurrently using the XML MPU API #1115
Conversation
andrewsg added the do not merge 
product-auto-label bot added size: l 
cojenco left a comment
cojenco left a comment There was a problem hiding this comment.
Looking good 🎉 have a few questions
andrewsg removed the do not merge | 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
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
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.
3 participants
This adds support for XML MPU multiprocess uploads to Transfer Manager.