[services][blob] Implement S3 multipart upload
ClosedPublic
Actions

Authored by bartek on Nov 18 2022, 9:57 AM.

Details

Reviewers

varun
• jon
tomek
marcin

Commits

rCOMMbd50208aed81: [services][blob] Implement S3 multipart upload

Summary

Implememted the MultipartUploadSession methods.
Implementation is based on:

Depends on D5679

Test Plan

Tested by calling these in local sandbox.
They will be tested extensively when the Put gRPC handler is implemented (have it locally, a few diffs ahead)

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

bartek created this revision.Nov 18 2022, 9:57 AM

bartek held this revision as a draft.

Herald added subscribers: atul, ashoat. · View Herald TranscriptNov 18 2022, 9:57 AM

Harbormaster completed remote builds in B13571: Diff 18590.Nov 18 2022, 10:08 AM

bartek added a child revision: D5681: [services][blob] Introduce database client.Nov 18 2022, 10:19 AM

bartek published this revision for review.Nov 18 2022, 10:42 AM

ashoat added 1 blocking reviewer(s): varun.Nov 18 2022, 12:58 PM

tomek added inline comments.Nov 22 2022, 7:24 AM

services/blob/src/s3.rs
132 ↗	(On Diff #18590)	We should avoid updating unrelated code in later diffs. If this change is necessary, it should be applied in the initial code.

The code seems to closely follow the official example code, which is good

varun accepted this revision.Nov 22 2022, 9:51 AM

varun added inline comments.

services/blob/src/s3.rs
89 ↗	(On Diff #18590)	do we plan on using this entity tag anywhere?

This revision is now accepted and ready to land.Nov 22 2022, 9:51 AM

bartek added inline comments.Nov 24 2022, 12:35 AM

services/blob/src/s3.rs
89 ↗	(On Diff #18590)	I don't know if it is necessary for uploads to work, but both C++ blob service and AWS example have this set.
132 ↗	(On Diff #18590)	Yeah, a formatter thing. I must've missed that. Such issues should be gone when we have a `cargo fmt` set in a pre-commit hook (there's a task for that)