adoroszlai commented on code in PR #3967:
URL: https://github.com/apache/ozone/pull/3967#discussion_r1186798671
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java:
##########
@@ -569,14 +569,24 @@ public final class OzoneConfigKeys {
OZONE_CLIENT_BUCKET_REPLICATION_CONFIG_REFRESH_PERIOD_DEFAULT_MS =
300 * 1000;
+
+ // Values for bucket layout configurations.
+ public static final String OZONE_BUCKET_LAYOUT_LEGACY =
+ "LEGACY";
+ public static final String OZONE_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED =
+ "FILE_SYSTEM_OPTIMIZED";
+ public static final String OZONE_BUCKET_LAYOUT_OBJECT_STORE =
+ "OBJECT_STORE";
+
public static final String OZONE_CLIENT_FS_DEFAULT_BUCKET_LAYOUT =
"ozone.client.fs.default.bucket.layout";
-
public static final String OZONE_CLIENT_FS_BUCKET_LAYOUT_DEFAULT =
- "FILE_SYSTEM_OPTIMIZED";
+ OZONE_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED;
- public static final String OZONE_CLIENT_FS_BUCKET_LAYOUT_LEGACY =
Review Comment:
Should we keep this constant for binary backward compatibility?
##########
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/OmUtils.java:
##########
@@ -509,6 +512,25 @@ public static void validateBucketName(String bucketName)
}
}
+ /**
+ * Verify bucket layout is a valid.
+ *
+ * @return The {@link BucketLayout} corresponding to the string.
+ * @throws ConfigurationException If the bucket layout is not valid.
+ */
+ public static BucketLayout validateBucketLayout(String bucketLayoutString) {
+ boolean bucketLayoutValid = Arrays.stream(BucketLayout.values())
+ .anyMatch(layout -> layout.name().equals(bucketLayoutString));
+ if (bucketLayoutValid) {
+ return BucketLayout.fromString(bucketLayoutString);
+ } else {
+ throw new ConfigurationException(bucketLayoutString +
+ " is not a valid default bucket layout. Supported values are " +
+ Arrays.stream(BucketLayout.values())
+ .map(Enum::toString).collect(Collectors.joining(", ")));
Review Comment:
Nit: validity check above uses `Enum#name()`, but exception message uses
`toString()`. These are equivalent by default, but `toString()` can be
overridden (e.g. for i18n/l10n purposes).
```suggestion
.map(Enum::name).collect(Collectors.joining(", ")));
```
Better yet, create a `SortedSet<String>` constant in `BucketLayout` with all
names, and use it in both validity check and exception message.
--
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]
