-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for multipart uploads #595
Conversation
1c795ca
to
6865776
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nothing but nits 👍 good stuff!!
return err | ||
} | ||
|
||
// TODO: f/u with support for 'prefix', 'keyMarker' and 'uploadIDMarker' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reminder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's on my list of things to f/u with.
MultipartListUploadsResponse struct { | ||
Uploads []MultipartListUploadItem `json:"uploads"` | ||
} | ||
MultipartListUploadItem struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is bucket
missing here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So it could be in MultipartListUploadsResponse
but I didn't add it since we don't need it to satisfy the corresponding S3 endpoint. We can just take the bucket from the request.
So etags are great, usually they're used to be able to return a 304 not modified response right (so the client can use its cached version), maybe we should extend |
return n, err | ||
} | ||
|
||
func (e *hashReader) Etag() string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit
Hash() string
?
This PR adds support for multipart uploads to the S3 API without storing the whole upload in memory.
NOTE: Migration code will be added and tested upon approval in case things still change.
Limitations
disablePreshardingEncryption
has to be 'true' when uploading a part. This will be fixed asap in a f/u with appropriate testing.