SteNicholas commented on a change in pull request #3633:
URL: https://github.com/apache/hudi/pull/3633#discussion_r706619802



##########
File path: 
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##########
@@ -136,10 +136,10 @@
       .defaultValue("archived")
       .withDocumentation("path under the meta folder, to store archived 
timeline instants at.");
 
-  public static final ConfigProperty<String> BOOTSTRAP_INDEX_ENABLE = 
ConfigProperty
+  public static final ConfigProperty<Boolean> BOOTSTRAP_INDEX_ENABLE = 
ConfigProperty
       .key("hoodie.bootstrap.index.enable")
-      .noDefaultValue()
-      .withDocumentation("Whether or not, this is a bootstrapped table, with 
bootstrap base data and an mapping index defined.");
+      .defaultValue(false)

Review comment:
       IMO, the default value of `hoodie.bootstrap.index.enable` should be true.

##########
File path: 
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##########
@@ -298,8 +298,9 @@ public String getBootstrapIndexClass() {
   }
 
   public static String getDefaultBootstrapIndexClass(Properties props) {
+    HoodieConfig hoodieConfig = new HoodieConfig(props);
     String defaultClass = BOOTSTRAP_INDEX_CLASS_NAME.defaultValue();
-    if 
("false".equalsIgnoreCase(props.getProperty(BOOTSTRAP_INDEX_ENABLE.key()))) {
+    if (!hoodieConfig.getBooleanOrDefault(BOOTSTRAP_INDEX_ENABLE)) {

Review comment:
       The option `hoodie.bootstrap.index.class` could not have the default 
value. If the default value of the `hoodie.bootstrap.index.class` is 
`HFileBootstrapIndex.class.getName()`, the method 
`getDefaultBootstrapIndexClass` should be renamed.




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