You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hudi.apache.org by GitBox <gi...@apache.org> on 2021/02/28 13:04:39 UTC

[GitHub] [hudi] nsivabalan commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For HoodieTableConfig

nsivabalan commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r584292305



##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##########
@@ -258,4 +260,164 @@ public String getArchivelogFolder() {
   public Properties getProperties() {
     return props;
   }
+
+  public static PropertyBuilder propertyBuilder() {

Review comment:
       is there any fixes or simplification required for HoodieTableConfig#createHoodieProperties(FileSystem fs, Path metadataFolder, Properties properties) ? 

##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##########
@@ -258,4 +260,164 @@ public String getArchivelogFolder() {
   public Properties getProperties() {
     return props;
   }
+
+  public static PropertyBuilder propertyBuilder() {
+    return new PropertyBuilder();
+  }
+
+  public static class PropertyBuilder {
+    private HoodieTableType tableType;
+    private String tableName;
+    private String archiveLogFolder;
+    private String payloadClassName;
+    private Integer timelineLayoutVersion;
+    private String baseFileFormat;
+    private String preCombineField;
+    private String bootstrapIndexClass;
+    private String bootstrapBasePath;
+
+    private PropertyBuilder() {
+
+    }
+
+    public PropertyBuilder setTableType(HoodieTableType tableType) {
+      this.tableType = tableType;
+      return this;
+    }
+
+    public PropertyBuilder setTableType(String tableType) {
+      return setTableType(HoodieTableType.valueOf(tableType));
+    }
+
+    public PropertyBuilder setTableName(String tableName) {
+      this.tableName = tableName;
+      return this;
+    }
+
+    public PropertyBuilder setArchiveLogFolder(String archiveLogFolder) {
+      this.archiveLogFolder = archiveLogFolder;
+      return this;
+    }
+
+    public PropertyBuilder setPayloadClassName(String payloadClassName) {
+      this.payloadClassName = payloadClassName;
+      return this;
+    }
+
+    public PropertyBuilder setPayloadClass(Class<? extends HoodieRecordPayload> payloadClass) {
+      return setPayloadClassName(payloadClass.getName());
+    }
+
+    public PropertyBuilder setTimelineLayoutVersion(Integer timelineLayoutVersion) {
+      this.timelineLayoutVersion = timelineLayoutVersion;
+      return this;
+    }
+
+    public PropertyBuilder setBaseFileFormat(String baseFileFormat) {
+      this.baseFileFormat = baseFileFormat;
+      return this;
+    }
+
+    public PropertyBuilder setPreCombineField(String preCombineField) {
+      this.preCombineField = preCombineField;
+      return this;
+    }
+
+    public PropertyBuilder setBootstrapIndexClass(String bootstrapIndexClass) {
+      this.bootstrapIndexClass = bootstrapIndexClass;
+      return this;
+    }
+
+    public PropertyBuilder setBootstrapBasePath(String bootstrapBasePath) {
+      this.bootstrapBasePath = bootstrapBasePath;
+      return this;
+    }
+
+    public PropertyBuilder fromMetaClient(HoodieTableMetaClient metaClient) {

Review comment:
       can you help point me to the place how/where this code exists prior to this patch? I checked HoodieTableMetaClient and couldn't find it. 

##########
File path: hudi-examples/src/main/java/org/apache/hudi/examples/java/HoodieJavaWriteClientExample.java
##########
@@ -72,8 +72,11 @@ public static void main(String[] args) throws Exception {
     Path path = new Path(tablePath);
     FileSystem fs = FSUtils.getFs(tablePath, hadoopConf);
     if (!fs.exists(path)) {
-      HoodieTableMetaClient.initTableType(hadoopConf, tablePath, HoodieTableType.valueOf(tableType),
-          tableName, HoodieAvroPayload.class.getName());
+      HoodieTableConfig.propertyBuilder()
+        .setTableType(tableType)
+        .setTableName(tableName)
+        .setPayloadClassName(HoodieAvroPayload.class.getName())
+        .initTable(hadoopConf, tablePath);

Review comment:
       this again makes me feel initTable() should move to HoodieTableMetaClient. bcoz, as you could see here, we start w/ instantiating HoodieTableConfig.propertyBuilder(), but final call results in HoodieTableMetaClient instance which does not sit well. 

##########
File path: hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##########
@@ -258,4 +260,164 @@ public String getArchivelogFolder() {
   public Properties getProperties() {
     return props;
   }
+
+  public static PropertyBuilder propertyBuilder() {
+    return new PropertyBuilder();
+  }
+
+  public static class PropertyBuilder {
+    private HoodieTableType tableType;
+    private String tableName;
+    private String archiveLogFolder;
+    private String payloadClassName;
+    private Integer timelineLayoutVersion;
+    private String baseFileFormat;
+    private String preCombineField;
+    private String bootstrapIndexClass;
+    private String bootstrapBasePath;
+
+    private PropertyBuilder() {
+
+    }
+
+    public PropertyBuilder setTableType(HoodieTableType tableType) {
+      this.tableType = tableType;
+      return this;
+    }
+
+    public PropertyBuilder setTableType(String tableType) {
+      return setTableType(HoodieTableType.valueOf(tableType));
+    }
+
+    public PropertyBuilder setTableName(String tableName) {
+      this.tableName = tableName;
+      return this;
+    }
+
+    public PropertyBuilder setArchiveLogFolder(String archiveLogFolder) {
+      this.archiveLogFolder = archiveLogFolder;
+      return this;
+    }
+
+    public PropertyBuilder setPayloadClassName(String payloadClassName) {
+      this.payloadClassName = payloadClassName;
+      return this;
+    }
+
+    public PropertyBuilder setPayloadClass(Class<? extends HoodieRecordPayload> payloadClass) {
+      return setPayloadClassName(payloadClass.getName());
+    }
+
+    public PropertyBuilder setTimelineLayoutVersion(Integer timelineLayoutVersion) {
+      this.timelineLayoutVersion = timelineLayoutVersion;
+      return this;
+    }
+
+    public PropertyBuilder setBaseFileFormat(String baseFileFormat) {
+      this.baseFileFormat = baseFileFormat;
+      return this;
+    }
+
+    public PropertyBuilder setPreCombineField(String preCombineField) {
+      this.preCombineField = preCombineField;
+      return this;
+    }
+
+    public PropertyBuilder setBootstrapIndexClass(String bootstrapIndexClass) {
+      this.bootstrapIndexClass = bootstrapIndexClass;
+      return this;
+    }
+
+    public PropertyBuilder setBootstrapBasePath(String bootstrapBasePath) {
+      this.bootstrapBasePath = bootstrapBasePath;
+      return this;
+    }
+
+    public PropertyBuilder fromMetaClient(HoodieTableMetaClient metaClient) {
+      return setTableType(metaClient.getTableType())
+        .setTableName(metaClient.getTableConfig().getTableName())
+        .setArchiveLogFolder(metaClient.getArchivePath())
+        .setPayloadClassName(metaClient.getTableConfig().getPayloadClass());
+    }
+
+    public PropertyBuilder fromProperties(Properties properties) {
+      if (properties.containsKey(HoodieTableConfig.HOODIE_TABLE_NAME_PROP_NAME)) {
+        setTableName(properties.getProperty(HoodieTableConfig.HOODIE_TABLE_NAME_PROP_NAME));
+      }
+      if (properties.containsKey(HoodieTableConfig.HOODIE_TABLE_TYPE_PROP_NAME)) {
+        setTableType(properties.getProperty(HoodieTableConfig.HOODIE_TABLE_TYPE_PROP_NAME));
+      }
+      if (properties.containsKey(HoodieTableConfig.HOODIE_ARCHIVELOG_FOLDER_PROP_NAME)) {
+        setArchiveLogFolder(properties.getProperty(HoodieTableConfig.HOODIE_ARCHIVELOG_FOLDER_PROP_NAME));
+      }
+      if (properties.containsKey(HoodieTableConfig.HOODIE_PAYLOAD_CLASS_PROP_NAME)) {
+        setPayloadClassName(properties.getProperty(HoodieTableConfig.HOODIE_PAYLOAD_CLASS_PROP_NAME));
+      }
+      if (properties.containsKey(HoodieTableConfig.HOODIE_TIMELINE_LAYOUT_VERSION)) {
+        setTimelineLayoutVersion(Integer.parseInt(properties.getProperty(HoodieTableConfig.HOODIE_TIMELINE_LAYOUT_VERSION)));
+      }
+      if (properties.containsKey(HoodieTableConfig.HOODIE_BASE_FILE_FORMAT_PROP_NAME)) {
+        setBaseFileFormat(properties.getProperty(HoodieTableConfig.HOODIE_BASE_FILE_FORMAT_PROP_NAME));
+      }
+      if (properties.containsKey(HoodieTableConfig.HOODIE_BOOTSTRAP_INDEX_CLASS_PROP_NAME)) {
+        setBootstrapIndexClass(properties.getProperty(HoodieTableConfig.HOODIE_BOOTSTRAP_INDEX_CLASS_PROP_NAME));
+      }
+      if (properties.containsKey(HoodieTableConfig.HOODIE_BOOTSTRAP_BASE_PATH)) {
+        setBootstrapBasePath(properties.getProperty(HoodieTableConfig.HOODIE_BOOTSTRAP_BASE_PATH));
+      }
+      if (properties.containsKey(HoodieTableConfig.HOODIE_TABLE_PRECOMBINE_FIELD)) {
+        setPreCombineField(properties.getProperty(HoodieTableConfig.HOODIE_TABLE_PRECOMBINE_FIELD));
+      }
+      return this;
+    }
+
+    public Properties build() {
+      ValidationUtils.checkArgument(tableType != null, "tableType is null");
+      ValidationUtils.checkArgument(tableName != null, "tableName is null");
+
+      Properties properties = new Properties();
+      properties.setProperty(HoodieTableConfig.HOODIE_TABLE_NAME_PROP_NAME, tableName);
+      properties.setProperty(HoodieTableConfig.HOODIE_TABLE_TYPE_PROP_NAME, tableType.name());
+      properties.setProperty(HoodieTableConfig.HOODIE_TABLE_VERSION_PROP_NAME, String.valueOf(HoodieTableVersion.current().versionCode()));
+      if (tableType == HoodieTableType.MERGE_ON_READ && payloadClassName != null) {
+        properties.setProperty(HoodieTableConfig.HOODIE_PAYLOAD_CLASS_PROP_NAME, payloadClassName);
+      }
+
+      if (null != archiveLogFolder) {
+        properties.put(HoodieTableConfig.HOODIE_ARCHIVELOG_FOLDER_PROP_NAME, archiveLogFolder);
+      }
+
+      if (null != timelineLayoutVersion) {
+        properties.put(HoodieTableConfig.HOODIE_TIMELINE_LAYOUT_VERSION, String.valueOf(timelineLayoutVersion));
+      }
+
+      if (null != baseFileFormat) {
+        properties.setProperty(HoodieTableConfig.HOODIE_BASE_FILE_FORMAT_PROP_NAME, baseFileFormat.toUpperCase());
+      }
+
+      if (null != bootstrapIndexClass) {
+        properties.put(HoodieTableConfig.HOODIE_BOOTSTRAP_INDEX_CLASS_PROP_NAME, bootstrapIndexClass);
+      }
+
+      if (null != bootstrapBasePath) {
+        properties.put(HoodieTableConfig.HOODIE_BOOTSTRAP_BASE_PATH, bootstrapBasePath);
+      }
+
+      if (null != preCombineField) {
+        properties.put(HoodieTableConfig.HOODIE_TABLE_PRECOMBINE_FIELD, preCombineField);
+      }
+      return properties;
+    }
+
+    /**
+     * Init Table with the properties build by this builder.
+     * @param configuration The hadoop config.
+     * @param basePath The base path for hoodie table.
+     * @return
+     * @throws IOException
+     */
+    public HoodieTableMetaClient initTable(Configuration configuration, String basePath) throws IOException {

Review comment:
       Wondering if we should leave this method in HoodieTableMetaClient itself ? 




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org