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 2020/10/04 17:10:35 UTC
[GitHub] [hudi] xushiyan opened a new pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
xushiyan opened a new pull request #2143:
URL: https://github.com/apache/hudi/pull/2143
## *Tips*
- *Thank you very much for contributing to Apache Hudi.*
- *Please review https://hudi.apache.org/contributing.html before opening a pull request.*
## What is the purpose of the pull request
*(For example: This pull request adds quick-start document.)*
## Brief change log
*(for example:)*
- *Modify AnnotationLocation checkstyle rule in checkstyle.xml*
## 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] codecov-io edited a comment on pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#issuecomment-705926236
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=h1) Report
> Merging [#2143](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hudi/commit/b335459c805748815ccc858ff1a9ef4cd830da8c?el=desc) will **increase** coverage by `6.23%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2143 +/- ##
============================================
+ Coverage 47.38% 53.61% +6.23%
- Complexity 2567 2845 +278
============================================
Files 358 359 +1
Lines 16484 16535 +51
Branches 1767 1777 +10
============================================
+ Hits 7811 8866 +1055
+ Misses 8032 6912 -1120
- Partials 641 757 +116
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| #hudicli | `38.37% <ø> (ø)` | `193.00 <ø> (ø)` | |
| #hudiclient | `100.00% <ø> (ø)` | `0.00 <ø> (ø)` | |
| #hudicommon | `54.74% <ø> (ø)` | `1793.00 <ø> (ø)` | |
| #hudihadoopmr | `33.05% <ø> (ø)` | `181.00 <ø> (ø)` | |
| #hudispark | `65.51% <ø> (ø)` | `303.00 <ø> (ø)` | |
| #huditimelineservice | `62.29% <ø> (-2.15%)` | `50.00 <ø> (+1.00)` | :arrow_down: |
| #hudiutilities | `69.98% <ø> (+59.30%)` | `325.00 <ø> (+277.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/2143?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../apache/hudi/timeline/service/TimelineService.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvVGltZWxpbmVTZXJ2aWNlLmphdmE=) | `25.00% <0.00%> (-2.54%)` | `6.00% <0.00%> (+1.00%)` | :arrow_down: |
| [.../hudi/utilities/schema/SchemaRegistryProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFSZWdpc3RyeVByb3ZpZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [...va/org/apache/hudi/utilities/schema/SchemaSet.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFTZXQuamF2YQ==) | `100.00% <0.00%> (ø)` | `3.00% <0.00%> (?%)` | |
| [.../apache/hudi/utilities/HoodieSnapshotExporter.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hvb2RpZVNuYXBzaG90RXhwb3J0ZXIuamF2YQ==) | `88.59% <0.00%> (+5.26%)` | `28.00% <0.00%> (ø%)` | |
| [...e/hudi/utilities/transform/ChainedTransformer.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3RyYW5zZm9ybS9DaGFpbmVkVHJhbnNmb3JtZXIuamF2YQ==) | `100.00% <0.00%> (+11.11%)` | `4.00% <0.00%> (+1.00%)` | |
| [...g/apache/hudi/utilities/schema/SchemaProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFQcm92aWRlci5qYXZh) | `100.00% <0.00%> (+20.00%)` | `2.00% <0.00%> (+1.00%)` | |
| [...in/java/org/apache/hudi/utilities/UtilHelpers.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL1V0aWxIZWxwZXJzLmphdmE=) | `64.59% <0.00%> (+27.95%)` | `30.00% <0.00%> (+19.00%)` | |
| [...ties/deltastreamer/HoodieDeltaStreamerMetrics.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllRGVsdGFTdHJlYW1lck1ldHJpY3MuamF2YQ==) | `36.11% <0.00%> (+36.11%)` | `6.00% <0.00%> (+6.00%)` | |
| [...ities/checkpointing/InitialCheckPointProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2NoZWNrcG9pbnRpbmcvSW5pdGlhbENoZWNrUG9pbnRQcm92aWRlci5qYXZh) | `45.45% <0.00%> (+45.45%)` | `1.00% <0.00%> (+1.00%)` | |
| [...di/utilities/sources/helpers/IncrSourceHelper.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9JbmNyU291cmNlSGVscGVyLmphdmE=) | `54.54% <0.00%> (+54.54%)` | `4.00% <0.00%> (+4.00%)` | |
| ... and [29 more](https://codecov.io/gh/apache/hudi/pull/2143/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 #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#issuecomment-705926236
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=h1) Report
> Merging [#2143](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hudi/commit/b335459c805748815ccc858ff1a9ef4cd830da8c?el=desc) will **increase** coverage by `6.23%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2143 +/- ##
============================================
+ Coverage 47.38% 53.61% +6.23%
- Complexity 2567 2845 +278
============================================
Files 358 359 +1
Lines 16484 16535 +51
Branches 1767 1777 +10
============================================
+ Hits 7811 8866 +1055
+ Misses 8032 6912 -1120
- Partials 641 757 +116
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| #hudicli | `38.37% <ø> (ø)` | `193.00 <ø> (ø)` | |
| #hudiclient | `100.00% <ø> (ø)` | `0.00 <ø> (ø)` | |
| #hudicommon | `54.74% <ø> (ø)` | `1793.00 <ø> (ø)` | |
| #hudihadoopmr | `33.05% <ø> (ø)` | `181.00 <ø> (ø)` | |
| #hudispark | `65.51% <ø> (ø)` | `303.00 <ø> (ø)` | |
| #huditimelineservice | `62.29% <ø> (-2.15%)` | `50.00 <ø> (+1.00)` | :arrow_down: |
| #hudiutilities | `69.98% <ø> (+59.30%)` | `325.00 <ø> (+277.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/2143?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../apache/hudi/timeline/service/TimelineService.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvVGltZWxpbmVTZXJ2aWNlLmphdmE=) | `25.00% <0.00%> (-2.54%)` | `6.00% <0.00%> (+1.00%)` | :arrow_down: |
| [.../hudi/utilities/schema/SchemaRegistryProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFSZWdpc3RyeVByb3ZpZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [...va/org/apache/hudi/utilities/schema/SchemaSet.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFTZXQuamF2YQ==) | `100.00% <0.00%> (ø)` | `3.00% <0.00%> (?%)` | |
| [.../apache/hudi/utilities/HoodieSnapshotExporter.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hvb2RpZVNuYXBzaG90RXhwb3J0ZXIuamF2YQ==) | `88.59% <0.00%> (+5.26%)` | `28.00% <0.00%> (ø%)` | |
| [...e/hudi/utilities/transform/ChainedTransformer.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3RyYW5zZm9ybS9DaGFpbmVkVHJhbnNmb3JtZXIuamF2YQ==) | `100.00% <0.00%> (+11.11%)` | `4.00% <0.00%> (+1.00%)` | |
| [...g/apache/hudi/utilities/schema/SchemaProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFQcm92aWRlci5qYXZh) | `100.00% <0.00%> (+20.00%)` | `2.00% <0.00%> (+1.00%)` | |
| [...in/java/org/apache/hudi/utilities/UtilHelpers.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL1V0aWxIZWxwZXJzLmphdmE=) | `64.59% <0.00%> (+27.95%)` | `30.00% <0.00%> (+19.00%)` | |
| [...ties/deltastreamer/HoodieDeltaStreamerMetrics.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllRGVsdGFTdHJlYW1lck1ldHJpY3MuamF2YQ==) | `36.11% <0.00%> (+36.11%)` | `6.00% <0.00%> (+6.00%)` | |
| [...ities/checkpointing/InitialCheckPointProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2NoZWNrcG9pbnRpbmcvSW5pdGlhbENoZWNrUG9pbnRQcm92aWRlci5qYXZh) | `45.45% <0.00%> (+45.45%)` | `1.00% <0.00%> (+1.00%)` | |
| [...di/utilities/sources/helpers/IncrSourceHelper.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9JbmNyU291cmNlSGVscGVyLmphdmE=) | `54.54% <0.00%> (+54.54%)` | `4.00% <0.00%> (+4.00%)` | |
| ... and [29 more](https://codecov.io/gh/apache/hudi/pull/2143/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] xushiyan commented on a change in pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#discussion_r502123493
##########
File path: hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieWriteableTestTable.java
##########
@@ -94,6 +106,10 @@ public String withInserts(String partition) throws Exception {
}
public String withInserts(String partition, HoodieRecord... records) throws Exception {
+ return withInserts(partition, Arrays.asList(records));
+ }
+
+ public String withInserts(String partition, List<HoodieRecord> records) throws Exception {
Review comment:
sounds good.
##########
File path: hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieWriteableTestTable.java
##########
@@ -128,4 +148,37 @@ public HoodieWriteableTestTable withInserts(String partition, String fileId, Hoo
return this;
}
+
+ public HoodieWriteableTestTable withLogAppends(HoodieRecord... records) throws Exception {
+ return withLogAppends(Arrays.asList(records));
+ }
+
+ public HoodieWriteableTestTable withLogAppends(List<HoodieRecord> records) throws Exception {
+ for (List<HoodieRecord> groupedRecords: records.stream()
+ .collect(Collectors.groupingBy(HoodieRecord::getCurrentLocation)).values()) {
+ appendRecordsToLogFile(groupedRecords);
+ }
+ return this;
+ }
+
+ private void appendRecordsToLogFile(List<HoodieRecord> groupedRecords) throws Exception {
+ String partitionPath = groupedRecords.get(0).getPartitionPath();
+ HoodieRecordLocation location = groupedRecords.get(0).getCurrentLocation();
+ try (HoodieLogFormat.Writer logWriter = HoodieLogFormat.newWriterBuilder().onParentPath(new Path(basePath, partitionPath))
+ .withFileExtension(HoodieLogFile.DELTA_EXTENSION).withFileId(location.getFileId())
+ .overBaseCommit(location.getInstantTime()).withFs(fs).build()) {
+ Map<HoodieLogBlock.HeaderMetadataType, String> header = new HashMap<>();
+ header.put(HoodieLogBlock.HeaderMetadataType.INSTANT_TIME, location.getInstantTime());
+ header.put(HoodieLogBlock.HeaderMetadataType.SCHEMA, schema.toString());
+ logWriter.appendBlock(new HoodieAvroDataBlock(groupedRecords.stream().map(r -> {
+ try {
+ GenericRecord val = (GenericRecord) r.getData().getInsertValue(schema).get();
+ HoodieAvroUtils.addHoodieKeyToRecord(val, r.getRecordKey(), r.getPartitionPath(), "");
+ return (IndexedRecord) val;
+ } catch (IOException e) {
Review comment:
@yanghua ok fixed.
----------------------------------------------------------------
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 #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#issuecomment-705926236
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=h1) Report
> Merging [#2143](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hudi/commit/b335459c805748815ccc858ff1a9ef4cd830da8c?el=desc) will **increase** coverage by `6.23%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2143 +/- ##
============================================
+ Coverage 47.38% 53.61% +6.23%
- Complexity 2567 2845 +278
============================================
Files 358 359 +1
Lines 16484 16535 +51
Branches 1767 1777 +10
============================================
+ Hits 7811 8866 +1055
+ Misses 8032 6912 -1120
- Partials 641 757 +116
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| #hudicli | `38.37% <ø> (ø)` | `193.00 <ø> (ø)` | |
| #hudiclient | `100.00% <ø> (ø)` | `0.00 <ø> (ø)` | |
| #hudicommon | `54.74% <ø> (ø)` | `1793.00 <ø> (ø)` | |
| #hudihadoopmr | `33.05% <ø> (ø)` | `181.00 <ø> (ø)` | |
| #hudispark | `65.51% <ø> (ø)` | `303.00 <ø> (ø)` | |
| #huditimelineservice | `62.29% <ø> (-2.15%)` | `50.00 <ø> (+1.00)` | :arrow_down: |
| #hudiutilities | `69.98% <ø> (+59.30%)` | `325.00 <ø> (+277.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/2143?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../apache/hudi/timeline/service/TimelineService.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvVGltZWxpbmVTZXJ2aWNlLmphdmE=) | `25.00% <0.00%> (-2.54%)` | `6.00% <0.00%> (+1.00%)` | :arrow_down: |
| [.../hudi/utilities/schema/SchemaRegistryProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFSZWdpc3RyeVByb3ZpZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [...va/org/apache/hudi/utilities/schema/SchemaSet.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFTZXQuamF2YQ==) | `100.00% <0.00%> (ø)` | `3.00% <0.00%> (?%)` | |
| [.../apache/hudi/utilities/HoodieSnapshotExporter.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hvb2RpZVNuYXBzaG90RXhwb3J0ZXIuamF2YQ==) | `88.59% <0.00%> (+5.26%)` | `28.00% <0.00%> (ø%)` | |
| [...e/hudi/utilities/transform/ChainedTransformer.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3RyYW5zZm9ybS9DaGFpbmVkVHJhbnNmb3JtZXIuamF2YQ==) | `100.00% <0.00%> (+11.11%)` | `4.00% <0.00%> (+1.00%)` | |
| [...g/apache/hudi/utilities/schema/SchemaProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFQcm92aWRlci5qYXZh) | `100.00% <0.00%> (+20.00%)` | `2.00% <0.00%> (+1.00%)` | |
| [...in/java/org/apache/hudi/utilities/UtilHelpers.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL1V0aWxIZWxwZXJzLmphdmE=) | `64.59% <0.00%> (+27.95%)` | `30.00% <0.00%> (+19.00%)` | |
| [...ties/deltastreamer/HoodieDeltaStreamerMetrics.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllRGVsdGFTdHJlYW1lck1ldHJpY3MuamF2YQ==) | `36.11% <0.00%> (+36.11%)` | `6.00% <0.00%> (+6.00%)` | |
| [...ities/checkpointing/InitialCheckPointProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2NoZWNrcG9pbnRpbmcvSW5pdGlhbENoZWNrUG9pbnRQcm92aWRlci5qYXZh) | `45.45% <0.00%> (+45.45%)` | `1.00% <0.00%> (+1.00%)` | |
| [...di/utilities/sources/helpers/IncrSourceHelper.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9JbmNyU291cmNlSGVscGVyLmphdmE=) | `54.54% <0.00%> (+54.54%)` | `4.00% <0.00%> (+4.00%)` | |
| ... and [29 more](https://codecov.io/gh/apache/hudi/pull/2143/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 #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#discussion_r502103169
##########
File path: hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieWriteableTestTable.java
##########
@@ -94,6 +106,10 @@ public String withInserts(String partition) throws Exception {
}
public String withInserts(String partition, HoodieRecord... records) throws Exception {
+ return withInserts(partition, Arrays.asList(records));
+ }
+
+ public String withInserts(String partition, List<HoodieRecord> records) throws Exception {
Review comment:
Considering the fluent APIs have one general pattern: method `withXXX` returns `HoodieWriteableTestTable ` itself. However, this method and above break this rule. WDYT about renaming them to `getFileIdWithInserts` or `returnFileIdWithInserts`?
##########
File path: hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieWriteableTestTable.java
##########
@@ -128,4 +148,37 @@ public HoodieWriteableTestTable withInserts(String partition, String fileId, Hoo
return this;
}
+
+ public HoodieWriteableTestTable withLogAppends(HoodieRecord... records) throws Exception {
+ return withLogAppends(Arrays.asList(records));
+ }
+
+ public HoodieWriteableTestTable withLogAppends(List<HoodieRecord> records) throws Exception {
+ for (List<HoodieRecord> groupedRecords: records.stream()
+ .collect(Collectors.groupingBy(HoodieRecord::getCurrentLocation)).values()) {
+ appendRecordsToLogFile(groupedRecords);
+ }
+ return this;
+ }
+
+ private void appendRecordsToLogFile(List<HoodieRecord> groupedRecords) throws Exception {
+ String partitionPath = groupedRecords.get(0).getPartitionPath();
+ HoodieRecordLocation location = groupedRecords.get(0).getCurrentLocation();
+ try (HoodieLogFormat.Writer logWriter = HoodieLogFormat.newWriterBuilder().onParentPath(new Path(basePath, partitionPath))
+ .withFileExtension(HoodieLogFile.DELTA_EXTENSION).withFileId(location.getFileId())
+ .overBaseCommit(location.getInstantTime()).withFs(fs).build()) {
+ Map<HoodieLogBlock.HeaderMetadataType, String> header = new HashMap<>();
+ header.put(HoodieLogBlock.HeaderMetadataType.INSTANT_TIME, location.getInstantTime());
+ header.put(HoodieLogBlock.HeaderMetadataType.SCHEMA, schema.toString());
+ logWriter.appendBlock(new HoodieAvroDataBlock(groupedRecords.stream().map(r -> {
+ try {
+ GenericRecord val = (GenericRecord) r.getData().getInsertValue(schema).get();
+ HoodieAvroUtils.addHoodieKeyToRecord(val, r.getRecordKey(), r.getPartitionPath(), "");
+ return (IndexedRecord) val;
+ } catch (IOException e) {
Review comment:
Logging the detailed exception information looks better?
----------------------------------------------------------------
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 #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
yanghua merged pull request #2143:
URL: https://github.com/apache/hudi/pull/2143
----------------------------------------------------------------
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] xushiyan commented on a change in pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#discussion_r499320666
##########
File path: hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestTable.java
##########
@@ -348,12 +372,36 @@ public String getBaseFileNameById(String fileId) {
return baseFileName(currentInstantTime, fileId);
}
- public List<FileStatus> listAllFiles(String partitionPath) throws IOException {
- return FileSystemTestUtils.listRecursive(fs, new Path(Paths.get(basePath, partitionPath).toString()));
+ public FileStatus[] listAllBaseFiles() throws IOException {
+ return listAllBaseFiles(HoodieFileFormat.PARQUET.getFileExtension());
+ }
+
+ public FileStatus[] listAllBaseFiles(String fileExtension) throws IOException {
+ return FileSystemTestUtils.listRecursive(fs, new Path(basePath)).stream()
+ .filter(status -> status.getPath().getName().endsWith(fileExtension))
+ .toArray(FileStatus[]::new);
+ }
+
+ public FileStatus[] listAllLogFiles() throws IOException {
+ return listAllLogFiles(HoodieFileFormat.HOODIE_LOG.getFileExtension());
+ }
+
+ public FileStatus[] listAllLogFiles(String fileExtension) throws IOException {
+ return FileSystemTestUtils.listRecursive(fs, new Path(basePath)).stream()
+ .filter(status -> status.getPath().getName().contains(fileExtension))
+ .toArray(FileStatus[]::new);
+ }
+
+ public FileStatus[] listAllBaseAndLogFiles() throws IOException {
+ return Stream.concat(Stream.of(listAllBaseFiles()), Stream.of(listAllLogFiles())).toArray(FileStatus[]::new);
+ }
+
+ public FileStatus[] listAllFilesInPartition(String partitionPath) throws IOException {
+ return FileSystemTestUtils.listRecursive(fs, new Path(Paths.get(basePath, partitionPath).toString())).toArray(new FileStatus[0]);
}
- public List<FileStatus> listAllFilesInTempFolder() throws IOException {
- return FileSystemTestUtils.listRecursive(fs, new Path(Paths.get(basePath, HoodieTableMetaClient.TEMPFOLDER_NAME).toString()));
+ public FileStatus[] listAllFilesInTempFolder() throws IOException {
+ return FileSystemTestUtils.listRecursive(fs, new Path(Paths.get(basePath, HoodieTableMetaClient.TEMPFOLDER_NAME).toString())).toArray(new FileStatus[0]);
Review comment:
change to return array to work with `HoodieTableFileSystemView` APIs which take in array
----------------------------------------------------------------
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 pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#issuecomment-705603856
@xushiyan Sorry for the late reply, the past week was during the National Day holiday, you know. Will review tomorrow.
----------------------------------------------------------------
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] xushiyan commented on pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
xushiyan commented on pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#issuecomment-703361805
@yanghua this should be the 2nd last PR of this migration. 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] codecov-io commented on pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#issuecomment-705926236
# [Codecov](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=h1) Report
> Merging [#2143](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hudi/commit/b335459c805748815ccc858ff1a9ef4cd830da8c?el=desc) will **increase** coverage by `6.23%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/hudi/pull/2143?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #2143 +/- ##
============================================
+ Coverage 47.38% 53.61% +6.23%
- Complexity 2567 2845 +278
============================================
Files 358 359 +1
Lines 16484 16535 +51
Branches 1767 1777 +10
============================================
+ Hits 7811 8866 +1055
+ Misses 8032 6912 -1120
- Partials 641 757 +116
```
| Flag | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| #hudicli | `38.37% <ø> (ø)` | `193.00 <ø> (ø)` | |
| #hudiclient | `100.00% <ø> (ø)` | `0.00 <ø> (ø)` | |
| #hudicommon | `54.74% <ø> (ø)` | `1793.00 <ø> (ø)` | |
| #hudihadoopmr | `33.05% <ø> (ø)` | `181.00 <ø> (ø)` | |
| #hudispark | `65.51% <ø> (ø)` | `303.00 <ø> (ø)` | |
| #huditimelineservice | `62.29% <ø> (-2.15%)` | `50.00 <ø> (+1.00)` | :arrow_down: |
| #hudiutilities | `69.98% <ø> (+59.30%)` | `325.00 <ø> (+277.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/2143?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [.../apache/hudi/timeline/service/TimelineService.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS10aW1lbGluZS1zZXJ2aWNlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9odWRpL3RpbWVsaW5lL3NlcnZpY2UvVGltZWxpbmVTZXJ2aWNlLmphdmE=) | `25.00% <0.00%> (-2.54%)` | `6.00% <0.00%> (+1.00%)` | :arrow_down: |
| [.../hudi/utilities/schema/SchemaRegistryProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFSZWdpc3RyeVByb3ZpZGVyLmphdmE=) | `0.00% <0.00%> (ø)` | `0.00% <0.00%> (ø%)` | |
| [...va/org/apache/hudi/utilities/schema/SchemaSet.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFTZXQuamF2YQ==) | `100.00% <0.00%> (ø)` | `3.00% <0.00%> (?%)` | |
| [.../apache/hudi/utilities/HoodieSnapshotExporter.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL0hvb2RpZVNuYXBzaG90RXhwb3J0ZXIuamF2YQ==) | `88.59% <0.00%> (+5.26%)` | `28.00% <0.00%> (ø%)` | |
| [...e/hudi/utilities/transform/ChainedTransformer.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3RyYW5zZm9ybS9DaGFpbmVkVHJhbnNmb3JtZXIuamF2YQ==) | `100.00% <0.00%> (+11.11%)` | `4.00% <0.00%> (+1.00%)` | |
| [...g/apache/hudi/utilities/schema/SchemaProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NjaGVtYS9TY2hlbWFQcm92aWRlci5qYXZh) | `100.00% <0.00%> (+20.00%)` | `2.00% <0.00%> (+1.00%)` | |
| [...in/java/org/apache/hudi/utilities/UtilHelpers.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL1V0aWxIZWxwZXJzLmphdmE=) | `64.59% <0.00%> (+27.95%)` | `30.00% <0.00%> (+19.00%)` | |
| [...ties/deltastreamer/HoodieDeltaStreamerMetrics.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2RlbHRhc3RyZWFtZXIvSG9vZGllRGVsdGFTdHJlYW1lck1ldHJpY3MuamF2YQ==) | `36.11% <0.00%> (+36.11%)` | `6.00% <0.00%> (+6.00%)` | |
| [...ities/checkpointing/InitialCheckPointProvider.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL2NoZWNrcG9pbnRpbmcvSW5pdGlhbENoZWNrUG9pbnRQcm92aWRlci5qYXZh) | `45.45% <0.00%> (+45.45%)` | `1.00% <0.00%> (+1.00%)` | |
| [...di/utilities/sources/helpers/IncrSourceHelper.java](https://codecov.io/gh/apache/hudi/pull/2143/diff?src=pr&el=tree#diff-aHVkaS11dGlsaXRpZXMvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2h1ZGkvdXRpbGl0aWVzL3NvdXJjZXMvaGVscGVycy9JbmNyU291cmNlSGVscGVyLmphdmE=) | `54.54% <0.00%> (+54.54%)` | `4.00% <0.00%> (+4.00%)` | |
| ... and [29 more](https://codecov.io/gh/apache/hudi/pull/2143/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] xushiyan commented on a change in pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#discussion_r499320189
##########
File path: hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieWriteableTestTable.java
##########
@@ -128,4 +148,37 @@ public HoodieWriteableTestTable withInserts(String partition, String fileId, Hoo
return this;
}
+
+ public HoodieWriteableTestTable withLogAppends(HoodieRecord... records) throws Exception {
+ return withLogAppends(Arrays.asList(records));
+ }
+
+ public HoodieWriteableTestTable withLogAppends(List<HoodieRecord> records) throws Exception {
+ for (List<HoodieRecord> groupedRecords: records.stream()
+ .collect(Collectors.groupingBy(HoodieRecord::getCurrentLocation)).values()) {
+ appendRecordsToLogFile(groupedRecords);
+ }
+ return this;
+ }
+
+ private void appendRecordsToLogFile(List<HoodieRecord> groupedRecords) throws Exception {
+ String partitionPath = groupedRecords.get(0).getPartitionPath();
+ HoodieRecordLocation location = groupedRecords.get(0).getCurrentLocation();
+ try (HoodieLogFormat.Writer logWriter = HoodieLogFormat.newWriterBuilder().onParentPath(new Path(basePath, partitionPath))
+ .withFileExtension(HoodieLogFile.DELTA_EXTENSION).withFileId(location.getFileId())
+ .overBaseCommit(location.getInstantTime()).withFs(fs).build()) {
+ Map<HoodieLogBlock.HeaderMetadataType, String> header = new HashMap<>();
+ header.put(HoodieLogBlock.HeaderMetadataType.INSTANT_TIME, location.getInstantTime());
+ header.put(HoodieLogBlock.HeaderMetadataType.SCHEMA, schema.toString());
+ logWriter.appendBlock(new HoodieAvroDataBlock(groupedRecords.stream().map(r -> {
+ try {
+ GenericRecord val = (GenericRecord) r.getData().getInsertValue(schema).get();
+ HoodieAvroUtils.addHoodieKeyToRecord(val, r.getRecordKey(), r.getPartitionPath(), "");
+ return (IndexedRecord) val;
+ } catch (IOException e) {
+ return null;
+ }
+ }).collect(Collectors.toList()), header));
Review comment:
refer to hudi-common/src/test/java/org/apache/hudi/common/testutils/HoodieTestUtils.java#writeRecordsToLogFiles
----------------------------------------------------------------
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] xushiyan commented on pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
xushiyan commented on pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#issuecomment-705608041
> @xushiyan Sorry for the late reply, the past week was during the National Day holiday, you know. Will review tomorrow.
@yanghua Understood. No worries.
----------------------------------------------------------------
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 #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
yanghua merged pull request #2143:
URL: https://github.com/apache/hudi/pull/2143
----------------------------------------------------------------
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] xushiyan commented on a change in pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#discussion_r502123493
##########
File path: hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieWriteableTestTable.java
##########
@@ -94,6 +106,10 @@ public String withInserts(String partition) throws Exception {
}
public String withInserts(String partition, HoodieRecord... records) throws Exception {
+ return withInserts(partition, Arrays.asList(records));
+ }
+
+ public String withInserts(String partition, List<HoodieRecord> records) throws Exception {
Review comment:
sounds good.
----------------------------------------------------------------
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] xushiyan commented on a change in pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
xushiyan commented on a change in pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#discussion_r502124874
##########
File path: hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieWriteableTestTable.java
##########
@@ -128,4 +148,37 @@ public HoodieWriteableTestTable withInserts(String partition, String fileId, Hoo
return this;
}
+
+ public HoodieWriteableTestTable withLogAppends(HoodieRecord... records) throws Exception {
+ return withLogAppends(Arrays.asList(records));
+ }
+
+ public HoodieWriteableTestTable withLogAppends(List<HoodieRecord> records) throws Exception {
+ for (List<HoodieRecord> groupedRecords: records.stream()
+ .collect(Collectors.groupingBy(HoodieRecord::getCurrentLocation)).values()) {
+ appendRecordsToLogFile(groupedRecords);
+ }
+ return this;
+ }
+
+ private void appendRecordsToLogFile(List<HoodieRecord> groupedRecords) throws Exception {
+ String partitionPath = groupedRecords.get(0).getPartitionPath();
+ HoodieRecordLocation location = groupedRecords.get(0).getCurrentLocation();
+ try (HoodieLogFormat.Writer logWriter = HoodieLogFormat.newWriterBuilder().onParentPath(new Path(basePath, partitionPath))
+ .withFileExtension(HoodieLogFile.DELTA_EXTENSION).withFileId(location.getFileId())
+ .overBaseCommit(location.getInstantTime()).withFs(fs).build()) {
+ Map<HoodieLogBlock.HeaderMetadataType, String> header = new HashMap<>();
+ header.put(HoodieLogBlock.HeaderMetadataType.INSTANT_TIME, location.getInstantTime());
+ header.put(HoodieLogBlock.HeaderMetadataType.SCHEMA, schema.toString());
+ logWriter.appendBlock(new HoodieAvroDataBlock(groupedRecords.stream().map(r -> {
+ try {
+ GenericRecord val = (GenericRecord) r.getData().getInsertValue(schema).get();
+ HoodieAvroUtils.addHoodieKeyToRecord(val, r.getRecordKey(), r.getPartitionPath(), "");
+ return (IndexedRecord) val;
+ } catch (IOException e) {
Review comment:
@yanghua ok fixed.
----------------------------------------------------------------
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 pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
yanghua commented on pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#issuecomment-705603856
@xushiyan Sorry for the late reply, the past week was during the National Day holiday, you know. Will review tomorrow.
----------------------------------------------------------------
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 #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
yanghua commented on a change in pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#discussion_r502103169
##########
File path: hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieWriteableTestTable.java
##########
@@ -94,6 +106,10 @@ public String withInserts(String partition) throws Exception {
}
public String withInserts(String partition, HoodieRecord... records) throws Exception {
+ return withInserts(partition, Arrays.asList(records));
+ }
+
+ public String withInserts(String partition, List<HoodieRecord> records) throws Exception {
Review comment:
Considering the fluent APIs have one general pattern: method `withXXX` returns `HoodieWriteableTestTable ` itself. However, this method and above break this rule. WDYT about renaming them to `getFileIdWithInserts` or `returnFileIdWithInserts`?
##########
File path: hudi-client/hudi-spark-client/src/test/java/org/apache/hudi/testutils/HoodieWriteableTestTable.java
##########
@@ -128,4 +148,37 @@ public HoodieWriteableTestTable withInserts(String partition, String fileId, Hoo
return this;
}
+
+ public HoodieWriteableTestTable withLogAppends(HoodieRecord... records) throws Exception {
+ return withLogAppends(Arrays.asList(records));
+ }
+
+ public HoodieWriteableTestTable withLogAppends(List<HoodieRecord> records) throws Exception {
+ for (List<HoodieRecord> groupedRecords: records.stream()
+ .collect(Collectors.groupingBy(HoodieRecord::getCurrentLocation)).values()) {
+ appendRecordsToLogFile(groupedRecords);
+ }
+ return this;
+ }
+
+ private void appendRecordsToLogFile(List<HoodieRecord> groupedRecords) throws Exception {
+ String partitionPath = groupedRecords.get(0).getPartitionPath();
+ HoodieRecordLocation location = groupedRecords.get(0).getCurrentLocation();
+ try (HoodieLogFormat.Writer logWriter = HoodieLogFormat.newWriterBuilder().onParentPath(new Path(basePath, partitionPath))
+ .withFileExtension(HoodieLogFile.DELTA_EXTENSION).withFileId(location.getFileId())
+ .overBaseCommit(location.getInstantTime()).withFs(fs).build()) {
+ Map<HoodieLogBlock.HeaderMetadataType, String> header = new HashMap<>();
+ header.put(HoodieLogBlock.HeaderMetadataType.INSTANT_TIME, location.getInstantTime());
+ header.put(HoodieLogBlock.HeaderMetadataType.SCHEMA, schema.toString());
+ logWriter.appendBlock(new HoodieAvroDataBlock(groupedRecords.stream().map(r -> {
+ try {
+ GenericRecord val = (GenericRecord) r.getData().getInsertValue(schema).get();
+ HoodieAvroUtils.addHoodieKeyToRecord(val, r.getRecordKey(), r.getPartitionPath(), "");
+ return (IndexedRecord) val;
+ } catch (IOException e) {
Review comment:
Logging the detailed exception information looks better?
----------------------------------------------------------------
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] xushiyan commented on pull request #2143: [HUDI-995] Migrate HoodieTestUtils APIs to HoodieTestTable
Posted by GitBox <gi...@apache.org>.
xushiyan commented on pull request #2143:
URL: https://github.com/apache/hudi/pull/2143#issuecomment-705608041
> @xushiyan Sorry for the late reply, the past week was during the National Day holiday, you know. Will review tomorrow.
@yanghua Understood. No worries.
----------------------------------------------------------------
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