spacemonkd commented on code in PR #10062:
URL: https://github.com/apache/ozone/pull/10062#discussion_r3621289797
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmMultipartPartInfo.java:
##########
@@ -70,9 +69,10 @@ private OmMultipartPartInfo(Builder b) {
if (b.partNumber <= 0) {
throw new IllegalArgumentException("partNumber is required and > 0");
}
- if (StringUtils.isBlank(b.eTag)) {
- throw new IllegalArgumentException("eTag is required");
- }
+ // eTag is optional: not all MPU clients supply an ETag at commit time
+ // (e.g. the Ozone native client), matching the legacy inline flow which
+ // never required it. It is stored when present and used for validation
+ // during CompleteMultipartUpload only when the client provides one.
Review Comment:
>Why is this making etags optional? I see we are keeping them required at
the gateway to be consistent with AWS but I don't understand why this change is
necessary in general or in the context of this PR.
So basically there are some tests under snapshot feature where the Ozone
client also tries multipart upload but without the ETags present.
Hence the original behaviour was that etags is optional, S3gateway should
enforce the mandatory etag, but OM doesn't need to enforce.
So we can either:
- Change the snapshot tests to remove those cases where null etag is passed
(probably raise a follow up JIRA and make ETag mandatory?)
- Or go ahead with what we did (i.e. make Etags optional) to avoid
regression.
> It should also be returned when the parts for an in-progress upload are
listed by the client.
This is already being done. `listParts` returns `partInfo.getETag();` for
the split schema (ref
[here](https://github.com/apache/ozone/blob/917e4b7706241317d8dbf7199f10be0cd43fdd7f/hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/KeyManagerImpl.java#L1204))
and also the legacy path returns it with the null fallback.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
