R2-3560: Implement ListParts and ListMultipartUploads for R2 Binding
#5596
Conversation
|
|
||
| KJ_IF_SOME(o, options) { | ||
| KJ_IF_SOME(l, o.limit) { | ||
| listBuilder.setLimit(l); |
There was a problem hiding this comment.
ListMultipartUploads should replicate the same validation logic we have for ListObjects I believe to be fully compliant with S3.
JSG_REQUIRE(l >= 1 && l <= 1000, RangeError,....
| kj::StringPtr components[1]; | ||
| auto path = fillR2Path(components, this->bucket->adminBucket); | ||
| CompatibilityFlags::Reader flags = {}; | ||
| auto promise = doR2HTTPGetRequest(kj::mv(client), kj::mv(requestJson), path, kj::none, flags); |
There was a problem hiding this comment.
Maybe I'm missing something, but wouldn't this fail if you are explicitly passing in kj::none instead of the JWT?
| } | ||
|
|
||
| struct R2ListMultipartUploadsRequest { | ||
| limit @0 :UInt32 = 0xffffffff; |
There was a problem hiding this comment.
Maybe I am just being paranoid, but would a more sensible default here make more sense (i.e AWS S3 defaults for max values?). IIRC 1000
| } | ||
| KJ_IF_SOME(p, o.partNumberMarker) { | ||
| JSG_REQUIRE( | ||
| p >= 0, RangeError, "partNumberMarker must be non-negative. Actual value was: ", p); |
There was a problem hiding this comment.
This is subtle and I learned this recently myself, but the part numbers are 1-indexed so this conditional is bugged since it needs to be strictly greater than.
| { | ||
| partNumber: 1, | ||
| etag: 'partEtag1', | ||
| size: '100', |
There was a problem hiding this comment.
Why are we treating size as a string? (Sorry I am no C++/kj expert so maybe it's normal, but it seems a bit odd)
| key: string; | ||
| uploadId: string; | ||
| initiated?: Date; | ||
| storageClass?: string; |
There was a problem hiding this comment.
Maybe strongly type the storage class with a default?
2 participants
First pass, may need polishing.