nsivabalan commented on code in PR #13950:
URL: https://github.com/apache/hudi/pull/13950#discussion_r2377906325
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/client/bootstrap/BootstrapRecordPayload.java:
##########
Review Comment:
is this used only for in memory processing? in other words, this payload may
not go into hoodie.properties for older versioned data table right?
CC @yihua
##########
hudi-utilities/src/test/java/org/apache/hudi/utilities/deltastreamer/HoodieDeltaStreamerTestBase.java:
##########
@@ -649,13 +646,6 @@ static HoodieDeltaStreamer.Config makeConfig(String
basePath, WriteOperationType
cfg.schemaProviderClassName = defaultSchemaProviderClassName;
}
cfg.allowCommitOnNoCheckpointChange = allowCommitOnNoCheckpointChange;
- Triple<RecordMergeMode, String, String> mergeCfgs =
- HoodieTableConfig.inferMergingConfigsForWrites(
- cfg.recordMergeMode, cfg.payloadClassName,
cfg.recordMergeStrategyId, cfg.sourceOrderingFields,
- HoodieTableVersion.current());
- cfg.recordMergeMode = mergeCfgs.getLeft();
- cfg.payloadClassName = mergeCfgs.getMiddle();
Review Comment:
we should add some tests for HoodieStreamer to test direct creation of v9
tables w/ some custom merge props.
for eg,
lets directly test AWSDMSAvroPayload.
i..e set corresponding props that would translate from AWSDMSAvroPayload and
validate ingestion succeeds and returns the expected result.
##########
hudi-utilities/src/test/java/org/apache/hudi/utilities/deltastreamer/HoodieDeltaStreamerTestBase.java:
##########
@@ -649,13 +646,6 @@ static HoodieDeltaStreamer.Config makeConfig(String
basePath, WriteOperationType
cfg.schemaProviderClassName = defaultSchemaProviderClassName;
}
cfg.allowCommitOnNoCheckpointChange = allowCommitOnNoCheckpointChange;
- Triple<RecordMergeMode, String, String> mergeCfgs =
- HoodieTableConfig.inferMergingConfigsForWrites(
- cfg.recordMergeMode, cfg.payloadClassName,
cfg.recordMergeStrategyId, cfg.sourceOrderingFields,
- HoodieTableVersion.current());
- cfg.recordMergeMode = mergeCfgs.getLeft();
- cfg.payloadClassName = mergeCfgs.getMiddle();
Review Comment:
if we remove this, does that mean that, all of existing HoodieStreamer tests
don't have any explicitly values set for these 3 merge related top level
configs?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
