[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
pengzhiwei2018 commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r583470709
##
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##
@@ -258,4 +260,142 @@ 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 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 Properties build() {
Review comment:
Hi @yanghua ,I have concerned about this question for same time. Because
there is already a `Builder` in the `HoodieTableMetaClient`. It is used to
create the MetaClient from an exists table.
This `builder` in HoodieTableConfig is just used to create table properties.
The initTable here is an alternative to the origin `initTableType` method.
And also there are some class call the `build() ` method .e.g.
[HDFSParquetImporter](https://github.com/apache/hudi/pull/2596/files#diff-a04828df2dc17403fdd6a8ead823d97213b0fdff62cfa0e0c515de349390d8c5)
and
[FunctionalTestHarness](https://github.com/apache/hudi/pull/2596/files#diff-3ced56d78ab4f4b7519fa8cd0a024cd6551a20d97155141bb20341745797ac62)
So I prefer to keep the build `method`, WDYT?
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:
us...@infra.apache.org
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
pengzhiwei2018 commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r583470709
##
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##
@@ -258,4 +260,142 @@ 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 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 Properties build() {
Review comment:
Hi @yanghua ,I have concerned about this question for same time. Because
there is already a `Builder` in the `HoodieTableMetaClient`. It is used to
create the MetaClient from an exists table.
This `builder` in HoodieTableConfig is just used to create table properties.
The initTable here is an alternative to the origin `initTableType` method.
And also there are some class call the `build() ` method .e.g.
[HDFSParquetImporter](https://github.com/apache/hudi/pull/2596/files#diff-a04828df2dc17403fdd6a8ead823d97213b0fdff62cfa0e0c515de349390d8c5)
So I prefer to keep the build `method`, WDYT?
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:
us...@infra.apache.org
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
pengzhiwei2018 commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r583471108
##
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##
@@ -258,4 +260,142 @@ public String getArchivelogFolder() {
public Properties getProperties() {
return props;
}
+
+ public static PropertyBuilder propertyBuilder() {
+return new PropertyBuilder();
+ }
+
+ public static class PropertyBuilder {
+
+private HoodieTableType tableType;
+
Review comment:
That is right.
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:
us...@infra.apache.org
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
pengzhiwei2018 commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r583470709
##
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##
@@ -258,4 +260,142 @@ 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 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 Properties build() {
Review comment:
Hi @yanghua ,I have concerned about this question for same time. Because
there is already a `Builder` in the `HoodieTableMetaClient`. It is used to
create the MetaClient from an exists table.
This `builder` in HoodieTableConfig is just used to create table properties.
The initTable here is an alternative to the origin `initTableType` method.
And also there are some class call the `build() ` method .e.g.
[HDFSParquetImporter](https://github.com/apache/hudi/pull/2596/files#diff-a04828df2dc17403fdd6a8ead823d97213b0fdff62cfa0e0c515de349390d8c5)
So I prefer to keep the build `method`.
##
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##
@@ -258,4 +260,142 @@ 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;
+
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
pengzhiwei2018 commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r583470709
##
File path:
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java
##
@@ -258,4 +260,142 @@ 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 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 Properties build() {
Review comment:
Hi @yanghua ,I have concerned about this question for same time. Because
there is already a `Builder` in the `HoodieTableMetaClient`. It is used to
create the MetaClient from an exists table.
This `builder` in HoodieTableConfig is just used to create table properties.
The initTable here is an alternative to the origin `initTableType` method.
And also there are some class call the `build() ` method .e.g.
[HDFSParquetImporter](https://github.com/apache/hudi/pull/2596/files#diff-a04828df2dc17403fdd6a8ead823d97213b0fdff62cfa0e0c515de349390d8c5)
So I prefer to keep the build method.
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:
us...@infra.apache.org
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
pengzhiwei2018 commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r583385132
##
File path: hudi-cli/src/main/java/org/apache/hudi/cli/commands/TableCommand.java
##
@@ -106,10 +106,13 @@ public String createTable(
throw new IllegalStateException("Table already existing in path : " +
path);
}
-final HoodieTableType tableType = HoodieTableType.valueOf(tableTypeStr);
-HoodieTableMetaClient.initTableType(HoodieCLI.conf, path, tableType, name,
archiveFolder,
-payloadClass, layoutVersion);
-
+new HoodieTableConfig.PropertyBuilder()
Review comment:
Hi @yanghua , I have update the PR. Please take a look again when you
have time. Thanks~
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:
us...@infra.apache.org
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
pengzhiwei2018 commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r583334728
##
File path: hudi-cli/src/main/java/org/apache/hudi/cli/commands/TableCommand.java
##
@@ -106,10 +106,13 @@ public String createTable(
throw new IllegalStateException("Table already existing in path : " +
path);
}
-final HoodieTableType tableType = HoodieTableType.valueOf(tableTypeStr);
-HoodieTableMetaClient.initTableType(HoodieCLI.conf, path, tableType, name,
archiveFolder,
-payloadClass, layoutVersion);
-
+new HoodieTableConfig.PropertyBuilder()
Review comment:
Sound good! I will have a try.
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:
us...@infra.apache.org
