yihua commented on code in PR #12868:
URL: https://github.com/apache/hudi/pull/12868#discussion_r1984179672
##########
hudi-utilities/src/test/java/org/apache/hudi/utilities/deltastreamer/TestHoodieDeltaStreamer.java:
##########
@@ -1877,20 +1874,11 @@ public void testPayloadClassUpdateWithCOWTable() throws
Exception {
assertTrue(props.containsKey(HoodieTableConfig.RECORD_MERGE_MODE.key()));
assertTrue(props.containsKey(HoodieTableConfig.RECORD_MERGE_STRATEGY_ID.key()));
- //now create one more deltaStreamer instance and update payload class
- cfg = TestHelpers.makeConfig(dataSetBasePath,
WriteOperationType.BULK_INSERT,
+ //now create one more deltaStreamer instance and try to update payload
class
+ HoodieDeltaStreamer.Config newCfg =
TestHelpers.makeConfig(dataSetBasePath, WriteOperationType.BULK_INSERT,
Collections.singletonList(SqlQueryBasedTransformer.class.getName()),
PROPS_FILENAME_TEST_SOURCE, false,
true, true, DummyAvroPayload.class.getName(), null);
- new HoodieDeltaStreamer(cfg, jsc, fs, hiveServer.getHiveConf());
-
- props = new Properties();
- fs = HadoopFSUtils.getFs(cfg.targetBasePath, jsc.hadoopConfiguration());
- try (InputStream inputStream = fs.open(new Path(metaPath))) {
- props.load(inputStream);
- }
-
- //now using payload
- assertEquals(DummyAvroPayload.class.getName(),
props.get(HoodieTableConfig.PAYLOAD_CLASS_NAME.key()));
+ assertThrows(HoodieValidationException.class, () -> new
HoodieDeltaStreamer(newCfg, jsc, fs, hiveServer.getHiveConf()));
Review Comment:
Could we add similar functional tests using Spark datasource and SQL writes
to ensure that merge configs cannot be changed if enabled?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -401,57 +402,29 @@ public static HoodieTableConfig
loadFromHoodieProps(HoodieStorage storage, Stora
}
private HoodieTableConfig(HoodieStorage storage, StoragePath metaPath) {
- this(storage, metaPath, null, null, null, false);
+ this(storage, metaPath, null, null, null);
}
public HoodieTableConfig(HoodieStorage storage, StoragePath metaPath,
RecordMergeMode recordMergeMode, String payloadClassName,
String recordMergeStrategyId) {
- this(storage, metaPath, recordMergeMode, payloadClassName,
recordMergeStrategyId, true);
Review Comment:
From usability standpoint, could we add a config to allow payload class or
merge mode/strategy update when enabled, and keep this config disabled by
default, i.e., not allowing record merge mode/strategy to change unless user
enables it in rare cases?
--
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]
