mukul1987 commented on a change in pull request #2357:
URL: https://github.com/apache/ozone/pull/2357#discussion_r656804141
##########
File path:
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/CreateBucketHandler.java
##########
@@ -50,6 +51,12 @@
"false/unspecified indicates otherwise")
private Boolean isGdprEnforced;
+ @Option(names = { "--type", "-t" },
Review comment:
This should be a ENUM based argument
##########
File path:
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreWithFSO.java
##########
@@ -98,27 +100,24 @@
*
* @throws IOException
*/
- @BeforeClass
- public static void init() throws Exception {
+ @BeforeClass public static void init() throws Exception {
conf = new OzoneConfiguration();
clusterId = UUID.randomUUID().toString();
scmId = UUID.randomUUID().toString();
omId = UUID.randomUUID().toString();
- TestOMRequestUtils.configureFSOptimizedPaths(conf,
- true, OMConfigKeys.OZONE_OM_METADATA_LAYOUT_PREFIX);
- cluster = MiniOzoneCluster.newBuilder(conf)
- .setClusterId(clusterId)
- .setScmId(scmId)
- .setOmId(omId)
- .build();
+ TestOMRequestUtils.configureFSOptimizedPaths(conf, true,
+ OMConfigKeys.OZONE_OM_METADATA_LAYOUT_PREFIX);
+ cluster = MiniOzoneCluster.newBuilder(conf).setClusterId(clusterId)
+ .setScmId(scmId).setOmId(omId).build();
cluster.waitForClusterToBeReady();
// create a volume and a bucket to be used by OzoneFileSystem
- OzoneBucket bucket = TestDataUtil.createVolumeAndBucket(cluster);
+ OzoneBucket bucket =
+ TestDataUtil.createVolumeAndBucket(cluster, BucketType.FSO);
volumeName = bucket.getVolumeName();
bucketName = bucket.getName();
- String rootPath = String.format("%s://%s.%s/",
- OzoneConsts.OZONE_URI_SCHEME, bucket.getName(),
+ String rootPath = String
+ .format("%s://%s.%s/", OzoneConsts.OZONE_URI_SCHEME, bucket.getName(),
Review comment:
Is this change needed
##########
File path:
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/helpers/OmBucketInfo.java
##########
@@ -523,6 +547,9 @@ public BucketInfo getProtobuf() {
.addAllMetadata(KeyValueUtil.toProtobuf(metadata))
.setQuotaInBytes(quotaInBytes)
.setQuotaInNamespace(quotaInNamespace);
+ if (bucketType != null) {
+ bib.setBucketType(bucketType.toProto());
+ }
Review comment:
Again, what the bucket type if the bucketType is null ?
##########
File path:
hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStoreWithFSO.java
##########
@@ -98,27 +100,24 @@
*
* @throws IOException
*/
- @BeforeClass
- public static void init() throws Exception {
+ @BeforeClass public static void init() throws Exception {
conf = new OzoneConfiguration();
clusterId = UUID.randomUUID().toString();
scmId = UUID.randomUUID().toString();
omId = UUID.randomUUID().toString();
- TestOMRequestUtils.configureFSOptimizedPaths(conf,
- true, OMConfigKeys.OZONE_OM_METADATA_LAYOUT_PREFIX);
- cluster = MiniOzoneCluster.newBuilder(conf)
- .setClusterId(clusterId)
- .setScmId(scmId)
- .setOmId(omId)
- .build();
+ TestOMRequestUtils.configureFSOptimizedPaths(conf, true,
Review comment:
This line should not be needed
##########
File path: hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
##########
@@ -528,6 +529,12 @@ enum StorageTypeProto {
RAM_DISK = 4;
}
+enum BucketTypeProto {
+ LEGACY = 1;
Review comment:
Should legacy be legacy filesystem here ?
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/BucketArgs.java
##########
@@ -61,6 +62,11 @@
private long quotaInBytes;
private long quotaInNamespace;
+ /**
+ * Bucket Type.
+ */
+ private BucketType bucketType;
Review comment:
we should have a default value for the bucket type, or is it a required
arg ?
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -201,6 +206,21 @@ public OzoneBucket(ConfigurationSource conf,
ClientProtocol proxy,
this.quotaInNamespace = quotaInNamespace;
}
+ @SuppressWarnings("parameternumber")
+ public OzoneBucket(ConfigurationSource conf, ClientProtocol proxy,
+ String volumeName, String bucketName, StorageType storageType,
+ Boolean versioning, long creationTime, long modificationTime,
+ Map<String, String> metadata, String encryptionKeyName,
+ String sourceVolume, String sourceBucket, long usedBytes,
+ long usedNamespace, long quotaInBytes, long quotaInNamespace,
+ BucketType bucketType) {
+ this(conf, proxy, volumeName, bucketName, storageType, versioning,
+ creationTime, modificationTime, metadata, encryptionKeyName,
+ sourceVolume, sourceBucket, usedBytes, usedNamespace, quotaInBytes,
+ quotaInNamespace);
+ this.bucketType = bucketType;
Review comment:
what will be the bucket type in the previous constructor ?
##########
File path:
hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/OzoneBucket.java
##########
@@ -563,7 +583,7 @@ public long getUsedNamespace() {
public Iterator<? extends OzoneKey> listKeys(String keyPrefix,
String prevKey) throws IOException {
- if(OzoneFSUtils.isFSOptimizedBucket(getMetadata())){
+ if(OzoneFSUtils.isFSOptimizedBucket(bucketType)){
Review comment:
should we not check bucketTypre.equals(FSO) or something like that ?
##########
File path:
hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/bucket/CreateBucketHandler.java
##########
@@ -60,9 +67,18 @@
public void execute(OzoneClient client, OzoneAddress address)
throws IOException {
- BucketArgs.Builder bb = new BucketArgs.Builder()
- .setStorageType(StorageType.DEFAULT)
- .setVersioning(false);
+ BucketArgs.Builder bb;
+ if (bucketType.equals(BucketType.OBJECT_STORE) || bucketType
Review comment:
https://picocli.info/#_enum_types
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
