rakeshadr commented on a change in pull request #2357:
URL: https://github.com/apache/ozone/pull/2357#discussion_r677082778
##########
File path: hadoop-ozone/interface-client/src/main/proto/OmClientProtocol.proto
##########
@@ -528,6 +529,12 @@ enum StorageTypeProto {
RAM_DISK = 4;
}
+enum BucketTypeProto {
Review comment:
Thanks a lot @marton for the comments.
> I am against removing the Map<String,String> as it can be used not only
for storing bucket types but storing any other metadata. Also, it's an
incompatible change if we remove it.
To make it clear about the`Map<String,String>` entity - this patch will not
remove `KeyValue` attribute from the BucketInfo and it will remain the same.
Any upcoming feature or behaviour can still make use of this `KeyValue`
attribute and build it.
_OmClientProtocol.proto_
```
message BucketInfo {
.....
repeated hadoop.hdds.KeyValue metadata = 7;
.....
+ optional BucketLayoutProto bucketLayout = 18;
```
This patch is moving the bucket layout information stored in the **Map to an
enum attribute.**
> Also, it's an incompatible change if we remove it.
IIUC, since the FSO(prefix) feature is not released in any of the Ozone
version we have still the opportunity to refactor/redesign the proto part.
Please let me know if I missed anything.
Following are the points while evaluating `enum` over `Map<String, String>`.
- enum has options to limit the set of valid values and will validate the
input efficiently. Please refer, https://picocli.info/#_enum_types
- protobuf defaults to the enum constant at ordinal 0 and not worrying about
null checks in a programatic way(easily avoid NPE cases). Not only from the
commandline, while reading back the old data from DB, where the bucket layout
will not exists in DB.
- according to me enum makes code more maintainable and easy to read
I'm completely open for a better approach. With map, we have to do all the
above in a programatic way and its doable. Please let me know your feedback.
Thanks again!
--
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]
