feat(services/s3): add append support#5428
Conversation
core/src/services/s3/core.rs
Outdated
| req = req.header(format!("{X_AMZ_META_PREFIX}{key}"), value) | ||
| } | ||
| } | ||
| req = self.insert_metadata_headers(req, Some(size), args); |
There was a problem hiding this comment.
Hi, I assume that only the first append call supports setting up metadata. We don't need to repeat the metadata for subsequent append requests.
There was a problem hiding this comment.
Hi, so sorry for the late reply! having two crazy weeks after the holidays :(
I may not follow what you mean but it seems we could set up the metadata after the first one?
There was a problem hiding this comment.
Hi! can I get some feedback here, really want to finish it!
There was a problem hiding this comment.
I think you're right. However, Maybe we don't need a new method s3_append_object_request. We can reuse the existing s3_put_object_request method. the only difference is that append requires a non-zero size.
There was a problem hiding this comment.
Thanks for the suggestion, but since I extract the meta data function out, I think keeping them separate is better for readability
There was a problem hiding this comment.
I may not follow what you mean but it seems we could set up the metadata after the first one?
Hi, from my current understanding, only the first append request can set metadata such as content-type and x-amz-meta-abc. Any metadata-related parameters in subsequent append requests will be ignored. That means that we can avoid sending those params in the subsequent append requests to save some extra bytes on the wire.
Xuanwo
left a comment
There was a problem hiding this comment.
Thank you @Frank-III for working on this!
Which issue does this PR close?
Close: #5351
Are there any user-facing changes?
when writing to S3 Express One Zone, user can: