smengcl commented on code in PR #11449:
URL: https://github.com/apache/ozone/pull/11449#discussion_r4310072638
##########
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/rpc/RpcClient.java:
##########
@@ -2340,6 +2340,23 @@ public OmMultipartUploadCompleteInfo
completeMultipartUpload(
omMultipartUploadCompleteList);
}
+ @Override
+ public void abortS3MultipartUpload(String bucketName, String keyName, String
uploadID) throws IOException {
+ if
(!OzoneManagerVersion.S3_MULTIPART_UPLOAD_ABORT_CONTEXT.isSupportedBy(omVersion))
{
+ abortMultipartUpload(getS3VolumeContext().getOmVolumeArgs().getVolume(),
bucketName, keyName, uploadID);
+ return;
+ }
+ verifyBucketName(bucketName);
+ HddsClientUtils.checkNotNull(keyName, uploadID);
+ OmKeyArgs keyArgs = new OmKeyArgs.Builder()
+ .setVolumeName(OzoneConfigKeys.OZONE_S3_VOLUME_NAME_DEFAULT)
Review Comment:
nit: Could we keep the comment from `getS3KeyInfoWithVolumeContext`
explaining the placeholder volume?
```suggestion
// Volume name is not important, as OM resolves the S3 volume when
assumeS3Context = true.
.setVolumeName(OzoneConfigKeys.OZONE_S3_VOLUME_NAME_DEFAULT)
```
##########
hadoop-ozone/client/src/test/java/org/apache/hadoop/ozone/client/rpc/TestRpcClient.java:
##########
@@ -359,6 +371,35 @@ public void testListMultipartUploadsValidatesNames(String
volume, String bucket,
}
}
+ @ParameterizedTest
+ @EnumSource(value = OzoneManagerVersion.class,
+ names = {"GET_FILE_STATUS_REJECTS_OBS", "ZDU",
"S3_MULTIPART_UPLOAD_ABORT_CONTEXT"})
+ void testAbortS3MultipartUpload(OzoneManagerVersion version) throws
Exception {
+ RpcClient client = spy(createRpcClient());
+ try {
+ OzoneManagerClientProtocol om = mock(OzoneManagerClientProtocol.class);
+ FieldUtils.writeField(client, "omVersion", version, true);
+ FieldUtils.writeField(client, "ozoneManagerClient", om, true);
+ doReturn(new
S3VolumeContext(OmVolumeArgs.newBuilder().setVolume("tenant-volume")
+ .setOwnerName("owner").setAdminName("admin").build(),
"owner")).when(client).getS3VolumeContext();
+ boolean assumeContext = version ==
OzoneManagerVersion.S3_MULTIPART_UPLOAD_ABORT_CONTEXT;
Review Comment:
Could this cover all versions, like
`testCreateKeyIfNotExistsRequiresSupportedOmVersion` below? The hand-picked
list skips `LIST_STATUS_REJECTS_OBS` (101), the version right below the new
one, and `version == S3_MULTIPART_UPLOAD_ABORT_CONTEXT` would take the wrong
branch once a newer OM version is added.
```suggestion
@EnumSource(value = OzoneManagerVersion.class, mode =
EnumSource.Mode.EXCLUDE, names = "UNKNOWN_VERSION")
void testAbortS3MultipartUpload(OzoneManagerVersion version) throws
Exception {
RpcClient client = spy(createRpcClient());
try {
OzoneManagerClientProtocol om = mock(OzoneManagerClientProtocol.class);
FieldUtils.writeField(client, "omVersion", version, true);
FieldUtils.writeField(client, "ozoneManagerClient", om, true);
doReturn(new
S3VolumeContext(OmVolumeArgs.newBuilder().setVolume("tenant-volume")
.setOwnerName("owner").setAdminName("admin").build(),
"owner")).when(client).getS3VolumeContext();
boolean assumeContext =
OzoneManagerVersion.S3_MULTIPART_UPLOAD_ABORT_CONTEXT.isSupportedBy(version);
```
--
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]
