ivandika3 commented on code in PR #9462:
URL: https://github.com/apache/ozone/pull/9462#discussion_r2652056991
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketCreateRequest.java:
##########
@@ -478,4 +506,25 @@ public static void addCreateVolumeToTable(String
volumeName,
.setOwnerName(UUID.randomUUID().toString()).build();
OMRequestTestUtils.addVolumeToOM(omMetadataManager, omVolumeArgs);
}
+
+ protected OMBucketCreateRequest doPreExecute(String volumeName,
+ String bucketName,
+ OzoneManagerProtocolProtos.BucketLayoutProto layout) throws Exception {
+ OzoneManagerProtocolProtos.BucketInfo.Builder bucketInfo =
+ newBucketInfoBuilder(bucketName, volumeName)
+ .setBucketLayout(layout);
+ if (layout ==
OzoneManagerProtocolProtos.BucketLayoutProto.FILE_SYSTEM_OPTIMIZED) {
+ bucketInfo.addMetadata(OMRequestTestUtils.fsoMetadata());
+ }
+ return doPreExecute(bucketInfo);
+ }
Review Comment:
I think this extra `doPreExecute` is not necessary. How about we simply use
the current `newBucketInfoBuilder#setBucketLayout(FILE_SYSTEM_OPTIMIZED)`? Also
the `addMetadata` is also not necessary.
##########
hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketCreateRequestWithFSO.java:
##########
@@ -182,4 +183,36 @@ protected void doValidateAndUpdateCache(String volumeName,
String bucketName,
verifySuccessCreateBucketResponse(omClientResponse.getOMResponse());
}
+
+ @Test
+ public void testNonS3BucketNameAllowedForFSOWhenStrictDisabled() throws
Exception {
+ // Arrange
+ ozoneManager.getConfiguration().setBoolean(
+ OMConfigKeys.OZONE_OM_NAMESPACE_STRICT_S3, false); // 如果常數名稱不同，改成實際的
key
Review Comment:
Nit: Please remove the mandarin comment (since some community members may
not understand it) and the "Arrange" comment also can be removed.
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketCreateRequest.java:
##########
@@ -93,9 +93,11 @@ public OMRequest preExecute(OzoneManager ozoneManager)
throws IOException {
CreateBucketRequest createBucketRequest =
getOmRequest().getCreateBucketRequest();
BucketInfo bucketInfo = createBucketRequest.getBucketInfo();
- // Verify resource name
- OmUtils.validateBucketName(bucketInfo.getBucketName(),
- ozoneManager.isStrictS3());
+
+ BucketLayout bucketLayout =
BucketLayout.fromProto(bucketInfo.getBucketLayout());
+ boolean strict = ozoneManager.isStrictS3()
+ ||
bucketLayout.isObjectStore(ozoneManager.getConfig().isFileSystemPathEnabled());
Review Comment:
Let's only check for OBS bucket (i.e. not use `BucketLayout#isObjectStore`
which also takes into account LEGACY buckets) since it provides a stronger
guarantee than LEGACY bucket which can become "object store" if we change the
configuration ("ozone.om.enable.filesystem.paths").
Also let's add a small comment that OBS bucket needs to have strictly S3
name bucket unlikes FSO and LEGACY bucket.
--
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]
