yihua commented on code in PR #11667:
URL: https://github.com/apache/hudi/pull/11667#discussion_r1690602593


##########
hudi-aws/src/main/java/org/apache/hudi/config/DynamoDbBasedLockConfig.java:
##########
@@ -43,20 +43,15 @@
         + " are auto managed internally.")
 public class DynamoDbBasedLockConfig extends HoodieConfig {
 
-  public static DynamoDbBasedLockConfig.Builder newBuilder() {
-    return new DynamoDbBasedLockConfig.Builder();
-  }
-
+  public static final int MAX_PARTITION_KEY_SIZE_BYTE = 2048;
   // configs for DynamoDb based locks
   public static final String DYNAMODB_BASED_LOCK_PROPERTY_PREFIX = 
LockConfiguration.LOCK_PREFIX + "dynamodb.";
-

Review Comment:
   Keep the empty lines?



##########
hudi-aws/src/main/java/org/apache/hudi/config/DynamoDbBasedLockConfig.java:
##########
@@ -140,26 +132,23 @@ public static class Builder {
 
     public DynamoDbBasedLockConfig build() {
       lockConfig.setDefaults(DynamoDbBasedLockConfig.class.getName());
-      checkRequiredProps(lockConfig);
       return lockConfig;
     }
 
     public Builder fromProperties(TypedProperties props) {
       lockConfig.getProps().putAll(props);
+      checkRequiredProps();

Review Comment:
   Should this be in `build()` after line 
`lockConfig.setDefaults(DynamoDbBasedLockConfig.class.getName());` because 
`fromProperties` might not be called?



##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieConfig.java:
##########
@@ -40,14 +40,13 @@
  */
 public class HoodieConfig implements Serializable {
 
-  private static final Logger LOG = 
LoggerFactory.getLogger(HoodieConfig.class);

Review Comment:
   Same here, keep the variable ordering?



##########
hudi-aws/src/main/java/org/apache/hudi/config/DynamoDbBasedLockConfig.java:
##########
@@ -43,20 +43,15 @@
         + " are auto managed internally.")
 public class DynamoDbBasedLockConfig extends HoodieConfig {
 
-  public static DynamoDbBasedLockConfig.Builder newBuilder() {
-    return new DynamoDbBasedLockConfig.Builder();
-  }
-

Review Comment:
   nit: don't change the order?



##########
hudi-common/src/main/java/org/apache/hudi/common/util/hash/HashID.java:
##########
@@ -30,8 +31,6 @@
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 
-import static org.apache.hudi.common.util.StringUtils.getUTF8Bytes;

Review Comment:
   Also keep the static import?



##########
hudi-common/src/main/java/org/apache/hudi/common/util/hash/HashID.java:
##########
@@ -41,52 +40,14 @@ public class HashID implements Serializable {
   private static final int HASH_SEED = 0xdabadaba;
 
   /**
-   * Represents HashID size in bits.

Review Comment:
   Same here on the ordering



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