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/23 12:39:49 UTC
[GitHub] [hudi] pengzhiwei2018 opened a new pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
pengzhiwei2018 opened a new pull request #2596:
URL: https://github.com/apache/hudi/pull/2596
…dieTableConfig
## What is the purpose of the pull request
Currently we use HoodieTableMetaClient#initTableType to init the table properties. As the number of Table Properties increases，It is hard to maintain the code for the initTableType method, especially when we add a new table property. Support Builder Pattern to build the table config properties can solve this problem.
## Brief change log
- Add `PropertyBuilder` to build the table properties and init the table.
- Remove the origin `initTableType` method in `HoodieMetaClient`.
- Refactor all the code refer to `initTableType`
## Verify this pull request
*(Please pick either of the following options)*
This pull request is a trivial rework / code cleanup without any test coverage.
*(or)*
This pull request is already covered by existing tests, such as *(please describe tests)*.
(or)
This change added tests and can be verified as follows:
*(example:)*
- *Added integration tests for end-to-end.*
- *Added HoodieClientWriteTest to verify the change.*
- *Manually verified the change by running a job locally.*
## Committer checklist
- [ ] Has a corresponding JIRA in PR title & commit
- [ ] Commit message is descriptive of the change
- [ ] CI is green
- [ ] Necessary doc changes done or have another open PR
- [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
----------------------------------------------------------------
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
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For HoodieTableConfig
Posted by GitBox <gi...@apache.org>.
pengzhiwei2018 commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r586224306
##########
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:
Hi @nsivabalan ,can we merge this PR? I am preparing another PR to solve the problem in [HUDI-1415](https://github.com/apache/hudi/pull/2283). This PR will add more table properties to the hoodie.properties when initTable. So I hope this PR can merge soon and I can use the `builder` in this PR. 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:
users@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…
Posted by GitBox <gi...@apache.org>.
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<? 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 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:
users@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…
Posted by GitBox <gi...@apache.org>.
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:
users@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…
Posted by GitBox <gi...@apache.org>.
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<? 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 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:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For HoodieTableConfig
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r588054717
##########
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:
Agree to move forward and do some subsequence refactor.
----------------------------------------------------------------
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
[GitHub] [hudi] codecov-io edited a comment on pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For HoodieTableConfig
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#issuecomment-784717451
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=h1) Report
> Merging [#2596](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=desc) (922a82f) into [master](https://codecov.io/gh/apache/hudi/commit/43a0776c7c88a5f7beac6c8853db7e341810635a?el=desc) (43a0776) will **increase** coverage by `18.36%`.
> The diff coverage is `69.56%`.
[](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2596 +/- ##
=============================================
+ Coverage 51.14% 69.50% +18.36%
+ Complexity 3215 364 -2851
=============================================
Files 438 53 -385
Lines 20041 1961 -18080
Branches 2064 235 -1829
=============================================
- Hits 10250 1363 -8887
+ Misses 8946 465 -8481
+ Partials 845 133 -712
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `?` | `?` | |
| hudiclient | `?` | `?` | |
| hudicommon | `?` | `?` | |
| hudiflink | `?` | `?` | |
| hudihadoopmr | `?` | `?` | |
| hudisparkdatasource | `?` | `?` | |
| hudisync | `?` | `?` | |
| huditimelineservice | `?` | `?` | |
| hudiutilities | `69.50% <69.56%> (+0.03%)` | `0.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `70.34% <50.00%> (+0.34%)` | `53.00 <0.00> (+3.00)` | |
| [...udi/utilities/deltastreamer/BootstrapExecutor.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvQm9vdHN0cmFwRXhlY3V0b3IuamF2YQ==) | `82.35% <100.00%> (+2.80%)` | `6.00 <0.00> (ø)` | |
| [...hudi/utilities/sources/helpers/KafkaOffsetGen.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9LYWZrYU9mZnNldEdlbi5qYXZh) | `85.84% <0.00%> (-2.94%)` | `20.00% <0.00%> (+4.00%)` | :arrow_down: |
| [...in/java/org/apache/hudi/utilities/UtilHelpers.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL1V0aWxIZWxwZXJzLmphdmE=) | `64.53% <0.00%> (-1.17%)` | `33.00% <0.00%> (+1.00%)` | :arrow_down: |
| [...i/common/table/log/block/HoodieHFileDataBlock.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL2xvZy9ibG9jay9Ib29kaWVIRmlsZURhdGFCbG9jay5qYXZh) | | | |
| [...a/org/apache/hudi/io/storage/HoodieFileReader.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaW8vc3RvcmFnZS9Ib29kaWVGaWxlUmVhZGVyLmphdmE=) | | | |
| [.../hadoop/utils/HoodieRealtimeRecordReaderUtils.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1oYWRvb3AtbXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaGFkb29wL3V0aWxzL0hvb2RpZVJlYWx0aW1lUmVjb3JkUmVhZGVyVXRpbHMuamF2YQ==) | | | |
| [.../common/util/queue/FunctionBasedQueueProducer.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvcXVldWUvRnVuY3Rpb25CYXNlZFF1ZXVlUHJvZHVjZXIuamF2YQ==) | | | |
| [...i-cli/src/main/java/org/apache/hudi/cli/Table.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL1RhYmxlLmphdmE=) | | | |
| [.../org/apache/hudi/cli/commands/TempViewCommand.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL2NvbW1hbmRzL1RlbXBWaWV3Q29tbWFuZC5qYXZh) | | | |
| ... and [376 more](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
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
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For HoodieTableConfig
Posted by GitBox <gi...@apache.org>.
pengzhiwei2018 commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r584539040
##########
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:
Hi @nsivabalan , I found many test case use an exist MetaClient to init the table.e.g. [TestHoodieClientOnCopyOnWriteStorage](https://github.com/apache/hudi/pull/2596/files#diff-e6700bb657614fd972f92e1cb7c74b0f95d5164c62e298bb4712bb70db9f8ada). So I provide this method to simplify the call.
----------------------------------------------------------------
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
[GitHub] [hudi] yanghua merged pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For HoodieTableConfig
Posted by GitBox <gi...@apache.org>.
yanghua merged pull request #2596:
URL: https://github.com/apache/hudi/pull/2596
----------------------------------------------------------------
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
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
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<? 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 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:
users@infra.apache.org
[GitHub] [hudi] nsivabalan commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For HoodieTableConfig
Posted by GitBox <gi...@apache.org>.
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
[GitHub] [hudi] codecov-io edited a comment on pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#issuecomment-784717451
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=h1) Report
> Merging [#2596](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=desc) (5d86b11) into [master](https://codecov.io/gh/apache/hudi/commit/43a0776c7c88a5f7beac6c8853db7e341810635a?el=desc) (43a0776) will **increase** coverage by `18.41%`.
> The diff coverage is `69.56%`.
[](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2596 +/- ##
=============================================
+ Coverage 51.14% 69.55% +18.41%
+ Complexity 3215 363 -2852
=============================================
Files 438 53 -385
Lines 20041 1961 -18080
Branches 2064 235 -1829
=============================================
- Hits 10250 1364 -8886
+ Misses 8946 463 -8483
+ Partials 845 134 -711
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `?` | `?` | |
| hudiclient | `?` | `?` | |
| hudicommon | `?` | `?` | |
| hudiflink | `?` | `?` | |
| hudihadoopmr | `?` | `?` | |
| hudisparkdatasource | `?` | `?` | |
| hudisync | `?` | `?` | |
| huditimelineservice | `?` | `?` | |
| hudiutilities | `69.55% <69.56%> (+0.09%)` | `0.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `70.00% <50.00%> (ø)` | `52.00 <0.00> (+2.00)` | |
| [...udi/utilities/deltastreamer/BootstrapExecutor.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvQm9vdHN0cmFwRXhlY3V0b3IuamF2YQ==) | `82.35% <100.00%> (+2.80%)` | `6.00 <0.00> (ø)` | |
| [...hudi/utilities/sources/helpers/KafkaOffsetGen.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9LYWZrYU9mZnNldEdlbi5qYXZh) | `85.84% <0.00%> (-2.94%)` | `20.00% <0.00%> (+4.00%)` | :arrow_down: |
| [...che/hudi/common/util/collection/ImmutablePair.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvY29sbGVjdGlvbi9JbW11dGFibGVQYWlyLmphdmE=) | | | |
| [...e/hudi/common/table/log/block/HoodieDataBlock.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL2xvZy9ibG9jay9Ib29kaWVEYXRhQmxvY2suamF2YQ==) | | | |
| [...sioning/clean/CleanMetadataV1MigrationHandler.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3RpbWVsaW5lL3ZlcnNpb25pbmcvY2xlYW4vQ2xlYW5NZXRhZGF0YVYxTWlncmF0aW9uSGFuZGxlci5qYXZh) | | | |
| [...he/hudi/cli/commands/HDFSParquetImportCommand.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL2NvbW1hbmRzL0hERlNQYXJxdWV0SW1wb3J0Q29tbWFuZC5qYXZh) | | | |
| [...a/org/apache/hudi/common/util/CollectionUtils.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvQ29sbGVjdGlvblV0aWxzLmphdmE=) | | | |
| [...di/common/table/log/block/HoodieAvroDataBlock.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL2xvZy9ibG9jay9Ib29kaWVBdnJvRGF0YUJsb2NrLmphdmE=) | | | |
| [...oning/compaction/CompactionV2MigrationHandler.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL3RpbWVsaW5lL3ZlcnNpb25pbmcvY29tcGFjdGlvbi9Db21wYWN0aW9uVjJNaWdyYXRpb25IYW5kbGVyLmphdmE=) | | | |
| ... and [374 more](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
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
[GitHub] [hudi] yanghua commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r583313615
##########
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:
WDYT about adding a static method named e.g. `HoodieTableConfig.propertyBuilder()` so that we can make the code more readable?
----------------------------------------------------------------
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
[GitHub] [hudi] codecov-io edited a comment on pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#issuecomment-784717451
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=h1) Report
> Merging [#2596](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=desc) (c71fe74) into [master](https://codecov.io/gh/apache/hudi/commit/43a0776c7c88a5f7beac6c8853db7e341810635a?el=desc) (43a0776) will **increase** coverage by `18.41%`.
> The diff coverage is `69.56%`.
[](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2596 +/- ##
=============================================
+ Coverage 51.14% 69.55% +18.41%
+ Complexity 3215 363 -2852
=============================================
Files 438 53 -385
Lines 20041 1961 -18080
Branches 2064 235 -1829
=============================================
- Hits 10250 1364 -8886
+ Misses 8946 463 -8483
+ Partials 845 134 -711
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `?` | `?` | |
| hudiclient | `?` | `?` | |
| hudicommon | `?` | `?` | |
| hudiflink | `?` | `?` | |
| hudihadoopmr | `?` | `?` | |
| hudisparkdatasource | `?` | `?` | |
| hudisync | `?` | `?` | |
| huditimelineservice | `?` | `?` | |
| hudiutilities | `69.55% <69.56%> (+0.09%)` | `0.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `70.00% <50.00%> (ø)` | `52.00 <0.00> (+2.00)` | |
| [...udi/utilities/deltastreamer/BootstrapExecutor.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvQm9vdHN0cmFwRXhlY3V0b3IuamF2YQ==) | `82.35% <100.00%> (+2.80%)` | `6.00 <0.00> (ø)` | |
| [...hudi/utilities/sources/helpers/KafkaOffsetGen.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9LYWZrYU9mZnNldEdlbi5qYXZh) | `85.84% <0.00%> (-2.94%)` | `20.00% <0.00%> (+4.00%)` | :arrow_down: |
| [...apache/hudi/timeline/service/handlers/Handler.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvaGFuZGxlcnMvSGFuZGxlci5qYXZh) | | | |
| [.../spark/sql/hudi/streaming/HoodieStreamSource.scala](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3Bhcmsvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9zcGFyay9zcWwvaHVkaS9zdHJlYW1pbmcvSG9vZGllU3RyZWFtU291cmNlLnNjYWxh) | | | |
| [.../java/org/apache/hudi/common/util/HoodieTimer.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvSG9vZGllVGltZXIuamF2YQ==) | | | |
| [...i/common/util/collection/ExternalSpillableMap.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvY29sbGVjdGlvbi9FeHRlcm5hbFNwaWxsYWJsZU1hcC5qYXZh) | | | |
| [...g/apache/hudi/common/table/log/LogReaderUtils.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL2xvZy9Mb2dSZWFkZXJVdGlscy5qYXZh) | | | |
| [...n/java/org/apache/hudi/common/HoodieCleanStat.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL0hvb2RpZUNsZWFuU3RhdC5qYXZh) | | | |
| [...del/OverwriteNonDefaultsWithLatestAvroPayload.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL21vZGVsL092ZXJ3cml0ZU5vbkRlZmF1bHRzV2l0aExhdGVzdEF2cm9QYXlsb2FkLmphdmE=) | | | |
| ... and [374 more](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
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
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For HoodieTableConfig
Posted by GitBox <gi...@apache.org>.
pengzhiwei2018 commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r584804328
##########
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:
Hi @nsivabalan @yanghua I have moved the `initTable` method and `propertyBuilder` to the `HoodieMetaClient` now. 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:
users@infra.apache.org
[GitHub] [hudi] yanghua commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r584110778
##########
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<? 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 Properties build() {
Review comment:
OK
----------------------------------------------------------------
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
[GitHub] [hudi] codecov-io edited a comment on pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For HoodieTableConfig
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#issuecomment-784717451
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=h1) Report
> Merging [#2596](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=desc) (922a82f) into [master](https://codecov.io/gh/apache/hudi/commit/43a0776c7c88a5f7beac6c8853db7e341810635a?el=desc) (43a0776) will **decrease** coverage by `41.60%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2596 +/- ##
============================================
- Coverage 51.14% 9.53% -41.61%
+ Complexity 3215 48 -3167
============================================
Files 438 53 -385
Lines 20041 1961 -18080
Branches 2064 235 -1829
============================================
- Hits 10250 187 -10063
+ Misses 8946 1761 -7185
+ Partials 845 13 -832
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `?` | `?` | |
| hudiclient | `?` | `?` | |
| hudicommon | `?` | `?` | |
| hudiflink | `?` | `?` | |
| hudihadoopmr | `?` | `?` | |
| hudisparkdatasource | `?` | `?` | |
| hudisync | `?` | `?` | |
| huditimelineservice | `?` | `?` | |
| hudiutilities | `9.53% <0.00%> (-59.94%)` | `0.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...udi/utilities/deltastreamer/BootstrapExecutor.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvQm9vdHN0cmFwRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (-79.55%)` | `0.00 <0.00> (-6.00)` | |
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `0.00% <0.00%> (-70.00%)` | `0.00 <0.00> (-50.00)` | |
| [...va/org/apache/hudi/utilities/IdentitySplitter.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0lkZW50aXR5U3BsaXR0ZXIuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-2.00%)` | |
| [...va/org/apache/hudi/utilities/schema/SchemaSet.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFTZXQuamF2YQ==) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-3.00%)` | |
| [...a/org/apache/hudi/utilities/sources/RowSource.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvUm93U291cmNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-4.00%)` | |
| [.../org/apache/hudi/utilities/sources/AvroSource.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvQXZyb1NvdXJjZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-1.00%)` | |
| [.../org/apache/hudi/utilities/sources/JsonSource.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvSnNvblNvdXJjZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-1.00%)` | |
| [...rg/apache/hudi/utilities/sources/CsvDFSSource.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvQ3N2REZTU291cmNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-10.00%)` | |
| [...g/apache/hudi/utilities/sources/JsonDFSSource.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvSnNvbkRGU1NvdXJjZS5qYXZh) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-4.00%)` | |
| [...apache/hudi/utilities/sources/JsonKafkaSource.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvSnNvbkthZmthU291cmNlLmphdmE=) | `0.00% <0.00%> (-100.00%)` | `0.00% <0.00%> (-6.00%)` | |
| ... and [413 more](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
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
[GitHub] [hudi] yanghua commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#discussion_r583407438
##########
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:
If we do not add comments, let us remove the empty line to make the code more compact. wdyt?
##########
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<? 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 Properties build() {
Review comment:
It seems we do not call this method out of the class. The major purpose of the inner class is to build a `HoodieTableMetaClient ` object? If yes, renaming to be the table mete client builder sounds more reasonable?
##########
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<? 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 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
Review comment:
Remove these elements or add more information?
----------------------------------------------------------------
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
[GitHub] [hudi] codecov-io edited a comment on pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#issuecomment-784717451
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=h1) Report
> Merging [#2596](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=desc) (025877c) into [master](https://codecov.io/gh/apache/hudi/commit/43a0776c7c88a5f7beac6c8853db7e341810635a?el=desc) (43a0776) will **increase** coverage by `18.30%`.
> The diff coverage is `69.56%`.
[](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2596 +/- ##
=============================================
+ Coverage 51.14% 69.45% +18.30%
+ Complexity 3215 363 -2852
=============================================
Files 438 53 -385
Lines 20041 1961 -18080
Branches 2064 235 -1829
=============================================
- Hits 10250 1362 -8888
+ Misses 8946 465 -8481
+ Partials 845 134 -711
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `?` | `?` | |
| hudiclient | `?` | `?` | |
| hudicommon | `?` | `?` | |
| hudiflink | `?` | `?` | |
| hudihadoopmr | `?` | `?` | |
| hudisparkdatasource | `?` | `?` | |
| hudisync | `?` | `?` | |
| huditimelineservice | `?` | `?` | |
| hudiutilities | `69.45% <69.56%> (-0.02%)` | `0.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `70.00% <50.00%> (ø)` | `52.00 <0.00> (+2.00)` | |
| [...udi/utilities/deltastreamer/BootstrapExecutor.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvQm9vdHN0cmFwRXhlY3V0b3IuamF2YQ==) | `82.35% <100.00%> (+2.80%)` | `6.00 <0.00> (ø)` | |
| [...hudi/utilities/sources/helpers/KafkaOffsetGen.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9LYWZrYU9mZnNldEdlbi5qYXZh) | `85.84% <0.00%> (-2.94%)` | `20.00% <0.00%> (+4.00%)` | :arrow_down: |
| [...in/java/org/apache/hudi/utilities/UtilHelpers.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL1V0aWxIZWxwZXJzLmphdmE=) | `64.53% <0.00%> (-1.17%)` | `33.00% <0.00%> (+1.00%)` | :arrow_down: |
| [...va/org/apache/hudi/metadata/BaseTableMetadata.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvbWV0YWRhdGEvQmFzZVRhYmxlTWV0YWRhdGEuamF2YQ==) | | | |
| [...main/java/org/apache/hudi/hive/HiveSyncConfig.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1zeW5jL2h1ZGktaGl2ZS1zeW5jL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2hpdmUvSGl2ZVN5bmNDb25maWcuamF2YQ==) | | | |
| [...rg/apache/hudi/hadoop/HoodieHFileRecordReader.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1oYWRvb3AtbXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvaGFkb29wL0hvb2RpZUhGaWxlUmVjb3JkUmVhZGVyLmphdmE=) | | | |
| [...a/org/apache/hudi/common/util/collection/Pair.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvY29sbGVjdGlvbi9QYWlyLmphdmE=) | | | |
| [...ava/org/apache/hudi/cli/commands/StatsCommand.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL2NvbW1hbmRzL1N0YXRzQ29tbWFuZC5qYXZh) | | | |
| [.../apache/hudi/common/util/ObjectSizeCalculator.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3V0aWwvT2JqZWN0U2l6ZUNhbGN1bGF0b3IuamF2YQ==) | | | |
| ... and [375 more](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
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
[GitHub] [hudi] codecov-io commented on pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#issuecomment-784717451
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=h1) Report
> Merging [#2596](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=desc) (b79eb00) into [master](https://codecov.io/gh/apache/hudi/commit/43a0776c7c88a5f7beac6c8853db7e341810635a?el=desc) (43a0776) will **decrease** coverage by `1.76%`.
> The diff coverage is `63.09%`.
[](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2596 +/- ##
============================================
- Coverage 51.14% 49.38% -1.77%
+ Complexity 3215 2846 -369
============================================
Files 438 381 -57
Lines 20041 17145 -2896
Branches 2064 1735 -329
============================================
- Hits 10250 8467 -1783
+ Misses 8946 8008 -938
+ Partials 845 670 -175
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `37.01% <100.00%> (+0.14%)` | `0.00 <0.00> (ø)` | |
| hudiclient | `100.00% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudicommon | `51.40% <48.93%> (+0.03%)` | `0.00 <0.00> (ø)` | |
| hudiflink | `46.46% <100.00%> (+1.02%)` | `0.00 <0.00> (ø)` | |
| hudihadoopmr | `33.16% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudisparkdatasource | `?` | `?` | |
| hudisync | `?` | `?` | |
| huditimelineservice | `?` | `?` | |
| hudiutilities | `69.47% <69.56%> (+<0.01%)` | `0.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...pache/hudi/common/table/HoodieTableMetaClient.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL0hvb2RpZVRhYmxlTWV0YUNsaWVudC5qYXZh) | `77.38% <ø> (+6.61%)` | `42.00 <0.00> (-2.00)` | :arrow_up: |
| [...rg/apache/hudi/common/table/HoodieTableConfig.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL0hvb2RpZVRhYmxlQ29uZmlnLmphdmE=) | `46.77% <48.93%> (+1.31%)` | `17.00 <0.00> (ø)` | |
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `69.31% <50.00%> (-0.69%)` | `50.00 <0.00> (ø)` | |
| [...ava/org/apache/hudi/cli/commands/TableCommand.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL2NvbW1hbmRzL1RhYmxlQ29tbWFuZC5qYXZh) | `87.80% <100.00%> (+1.69%)` | `7.00 <0.00> (ø)` | |
| [...c/main/java/org/apache/hudi/util/StreamerUtil.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS91dGlsL1N0cmVhbWVyVXRpbC5qYXZh) | `41.50% <100.00%> (+1.12%)` | `12.00 <0.00> (ø)` | |
| [...udi/utilities/deltastreamer/BootstrapExecutor.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvQm9vdHN0cmFwRXhlY3V0b3IuamF2YQ==) | `82.35% <100.00%> (+2.80%)` | `6.00 <0.00> (ø)` | |
| [...udi/operator/partitioner/BucketAssignFunction.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS9vcGVyYXRvci9wYXJ0aXRpb25lci9CdWNrZXRBc3NpZ25GdW5jdGlvbi5qYXZh) | `79.54% <0.00%> (-12.77%)` | `18.00% <0.00%> (+10.00%)` | :arrow_down: |
| [...ain/java/org/apache/hudi/avro/HoodieAvroUtils.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvYXZyby9Ib29kaWVBdnJvVXRpbHMuamF2YQ==) | `55.66% <0.00%> (-0.44%)` | `38.00% <0.00%> (ø%)` | |
| [...org/apache/hudi/HoodieDatasetBulkInsertHelper.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3Bhcmsvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvSG9vZGllRGF0YXNldEJ1bGtJbnNlcnRIZWxwZXIuamF2YQ==) | | | |
| [...rc/main/scala/org/apache/hudi/Spark3RowSerDe.scala](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3BhcmszL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvaHVkaS9TcGFyazNSb3dTZXJEZS5zY2FsYQ==) | | | |
| ... and [55 more](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
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
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
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<? 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 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;
+ 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 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
Review comment:
ok
----------------------------------------------------------------
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
[GitHub] [hudi] pengzhiwei2018 commented on a change in pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org
[GitHub] [hudi] codecov-io edited a comment on pull request #2596: [HUDI-1636] Support Builder Pattern To Build Table Properties For Hoo…
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2596:
URL: https://github.com/apache/hudi/pull/2596#issuecomment-784717451
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=h1) Report
> Merging [#2596](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=desc) (b79eb00) into [master](https://codecov.io/gh/apache/hudi/commit/43a0776c7c88a5f7beac6c8853db7e341810635a?el=desc) (43a0776) will **increase** coverage by `0.12%`.
> The diff coverage is `69.00%`.
[](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2596 +/- ##
============================================
+ Coverage 51.14% 51.26% +0.12%
- Complexity 3215 3224 +9
============================================
Files 438 438
Lines 20041 20142 +101
Branches 2064 2070 +6
============================================
+ Hits 10250 10326 +76
- Misses 8946 8967 +21
- Partials 845 849 +4
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| hudicli | `37.01% <100.00%> (+0.14%)` | `0.00 <0.00> (ø)` | |
| hudiclient | `100.00% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudicommon | `51.40% <48.93%> (+0.03%)` | `0.00 <0.00> (ø)` | |
| hudiflink | `46.46% <100.00%> (+1.02%)` | `0.00 <0.00> (ø)` | |
| hudihadoopmr | `33.16% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudisparkdatasource | `69.92% <100.00%> (+0.17%)` | `0.00 <0.00> (ø)` | |
| hudisync | `48.61% <ø> (ø)` | `0.00 <ø> (ø)` | |
| huditimelineservice | `66.49% <ø> (ø)` | `0.00 <ø> (ø)` | |
| hudiutilities | `69.47% <69.56%> (+<0.01%)` | `0.00 <0.00> (ø)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/hudi/pull/2596?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...pache/hudi/common/table/HoodieTableMetaClient.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL0hvb2RpZVRhYmxlTWV0YUNsaWVudC5qYXZh) | `77.38% <ø> (+6.61%)` | `42.00 <0.00> (-2.00)` | :arrow_up: |
| [...rg/apache/hudi/common/table/HoodieTableConfig.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY29tbW9uL3RhYmxlL0hvb2RpZVRhYmxlQ29uZmlnLmphdmE=) | `46.77% <48.93%> (+1.31%)` | `17.00 <0.00> (ø)` | |
| [...apache/hudi/utilities/deltastreamer/DeltaSync.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvRGVsdGFTeW5jLmphdmE=) | `69.31% <50.00%> (-0.69%)` | `50.00 <0.00> (ø)` | |
| [...ava/org/apache/hudi/cli/commands/TableCommand.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jbGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvY2xpL2NvbW1hbmRzL1RhYmxlQ29tbWFuZC5qYXZh) | `87.80% <100.00%> (+1.69%)` | `7.00 <0.00> (ø)` | |
| [...c/main/java/org/apache/hudi/util/StreamerUtil.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS91dGlsL1N0cmVhbWVyVXRpbC5qYXZh) | `41.50% <100.00%> (+1.12%)` | `12.00 <0.00> (ø)` | |
| [...n/scala/org/apache/hudi/HoodieSparkSqlWriter.scala](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1zcGFyay1kYXRhc291cmNlL2h1ZGktc3Bhcmsvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9odWRpL0hvb2RpZVNwYXJrU3FsV3JpdGVyLnNjYWxh) | `51.53% <100.00%> (+1.53%)` | `0.00 <0.00> (ø)` | |
| [...udi/utilities/deltastreamer/BootstrapExecutor.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvQm9vdHN0cmFwRXhlY3V0b3IuamF2YQ==) | `82.35% <100.00%> (+2.80%)` | `6.00 <0.00> (ø)` | |
| [...udi/operator/partitioner/BucketAssignFunction.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1mbGluay9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaHVkaS9vcGVyYXRvci9wYXJ0aXRpb25lci9CdWNrZXRBc3NpZ25GdW5jdGlvbi5qYXZh) | `79.54% <0.00%> (-12.77%)` | `18.00% <0.00%> (+10.00%)` | :arrow_down: |
| [...ain/java/org/apache/hudi/avro/HoodieAvroUtils.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1jb21tb24vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvYXZyby9Ib29kaWVBdnJvVXRpbHMuamF2YQ==) | `55.66% <0.00%> (-0.44%)` | `38.00% <0.00%> (ø%)` | |
| [...c/main/java/org/apache/hudi/hive/HiveSyncTool.java](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree#diff-aHVkaS1zeW5jL2h1ZGktaGl2ZS1zeW5jL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL2hpdmUvSGl2ZVN5bmNUb29sLmphdmE=) | `69.07% <0.00%> (ø)` | `14.00% <0.00%> (ø%)` | |
| ... and [1 more](https://codecov.io/gh/apache/hudi/pull/2596/diff?src=pr&el=tree-more) | |
----------------------------------------------------------------
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