Dzeri96 commented on code in PR #3597:
URL: https://github.com/apache/celeborn/pull/3597#discussion_r2811698980


##########
common/src/main/java/org/apache/celeborn/common/protocol/StorageInfo.java:
##########
@@ -22,22 +22,34 @@
 
 public class StorageInfo implements Serializable {
   public enum Type {
-    MEMORY(0),
-    HDD(1),
-    SSD(2),
-    HDFS(3),
-    OSS(4),
-    S3(5);
+    MEMORY(0, false, MEMORY_MASK),
+    HDD(1, false, LOCAL_DISK_MASK),
+    SSD(2, false, LOCAL_DISK_MASK),
+    HDFS(3, true, HDFS_MASK),
+    OSS(4, true, OSS_MASK),
+    S3(5, true, S3_MASK);
 
     private final int value;
+    private final boolean isDFS;
+    private final int mask;
 
-    Type(int value) {
+    Type(int value, boolean isDFS, int mask) {
       this.value = value;
+      this.isDFS = isDFS;

Review Comment:
   So my initial idea for implementing this was a HashMap. I had built a map 
that was being filled in the `static` block, like the other maps, but then I 
realized you had to make sure each ENUM member was in this map, so I wrote a 
test to enforce it.
   In the end I found this static solution much more elegant. The compiler 
forces you to assign each enum member a isDFS value. It's also less code. Let 
me know if you want me to change it though.



-- 
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]

Reply via email to