Sync APIs with S3 specification #1678
Conversation
balamurugana
force-pushed
the
Sync-APIs-with-S3-specification
branch
5 times, most recently
from
7cb505b to
97cdc0a
Compare
balamurugana
force-pushed
the
Sync-APIs-with-S3-specification
branch
5 times, most recently
from
a186c07 to
29c4a9b
Compare
kannappanr
requested review from
anjalshireesh,
shtripat and
sivanantham-gnanavel
anjalshireesh
left a comment
anjalshireesh
left a comment
There was a problem hiding this comment.
LGTM, one minor nit and a clarification.
balamurugana
force-pushed
the
Sync-APIs-with-S3-specification
branch
2 times, most recently
from
b8c0c94 to
ef2a2fe
Compare
| CopyObjectResult result) { | ||
| super(headers, bucket, region, object); | ||
| this.etag = etag; | ||
| this.lastModified = result.lastModified(); |
There was a problem hiding this comment.
result called before null check. so, may be a NPE.
|
|
||
| protected ObjectConditionalReadArgs(ObjectConditionalReadArgs args, String matchETag) { | ||
| this(args); | ||
| this.matchETag = args.matchETag; |
There was a problem hiding this comment.
ignores parameter matchETag. so, Do we need matchETag parameter?
| config = | ||
| new CreateBucketConfiguration( | ||
| locationConstraint, args.locationConfig(), args.bucketConfig()); | ||
| locationConstraint, args.locationConfig(), args.bucketConfig(), args.tags()); |
There was a problem hiding this comment.
Small doubt here. Tags are only passed to US_EAST_1. What about other regions? Don't we need to pass it?
|
|
||
| protected ObjectConditionalReadArgs(SourceObject args) { | ||
| super(args); | ||
| this.matchETag = args.matchETag(); |
There was a problem hiding this comment.
Similarly, We need to add the same to the constructor of GetObjectArgs.java
| if (algorithms != null) algorithm = algorithms.get(0); | ||
| Map<Checksum.Algorithm, String> checksums = response.checksums(); | ||
| if (checksums != null && !checksums.isEmpty()) { | ||
| algorithm = checksums.keySet().iterator().next(); |
There was a problem hiding this comment.
HashMap does not guarantee iteration order. If object has two or more checksums then,.next() will return unpredictable result. Try using LinkedHashMap or EnumMap.
if the order is not an issue then ignore this comment.
Co-authored-by: Shireesh Anjal <355479+anjalshireesh@users.noreply.github.com> Co-authored-by: Sivanantham Gnanavel <158702293+sivanantham-gnanavel@users.noreply.github.com> Signed-off-by: Bala.FA <bala@minio.io>
balamurugana
force-pushed
the
Sync-APIs-with-S3-specification
branch
from
bd60a4e to
3273c9f
Compare
4 participants
No description provided.