codope commented on code in PR #6073:
URL: https://github.com/apache/hudi/pull/6073#discussion_r917345010
##########
hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/HoodieSyncConfig.java:
##########
@@ -56,22 +59,14 @@ public class HoodieSyncConfig extends HoodieConfig {
public static final ConfigProperty<String> META_SYNC_DATABASE_NAME =
ConfigProperty
.key("hoodie.datasource.hive_sync.database")
.defaultValue("default")
+ .withInferFunction(cfg ->
Option.ofNullable(cfg.getString(DATABASE_NAME)))
.withDocumentation("The name of the destination database that we should
sync the hudi table to.");
- // If the table name for the metastore destination is not provided, pick it
up from write or table configs.
- public static final Function<HoodieConfig, Option<String>>
TABLE_NAME_INFERENCE_FUNCTION = cfg -> {
- if (cfg.contains(HoodieTableConfig.HOODIE_WRITE_TABLE_NAME_KEY)) {
- return
Option.of(cfg.getString(HoodieTableConfig.HOODIE_WRITE_TABLE_NAME_KEY));
- } else if (cfg.contains(HoodieTableConfig.HOODIE_TABLE_NAME_KEY)) {
- return Option.of(cfg.getString(HoodieTableConfig.HOODIE_TABLE_NAME_KEY));
- } else {
- return Option.empty();
- }
- };
public static final ConfigProperty<String> META_SYNC_TABLE_NAME =
ConfigProperty
.key("hoodie.datasource.hive_sync.table")
.defaultValue("unknown")
- .withInferFunction(TABLE_NAME_INFERENCE_FUNCTION)
+ .withInferFunction(cfg ->
Option.ofNullable(cfg.getString(HOODIE_WRITE_TABLE_NAME_KEY))
Review Comment:
I think we should remove setting defaultValue if the config is being
inferred from a mandatory config so that the app can fail fast. What do you
think?
##########
hudi-sync/hudi-sync-common/src/main/java/org/apache/hudi/sync/common/HoodieSyncConfig.java:
##########
@@ -56,22 +59,14 @@ public class HoodieSyncConfig extends HoodieConfig {
public static final ConfigProperty<String> META_SYNC_DATABASE_NAME =
ConfigProperty
.key("hoodie.datasource.hive_sync.database")
.defaultValue("default")
+ .withInferFunction(cfg ->
Option.ofNullable(cfg.getString(DATABASE_NAME)))
Review Comment:
We should note that `ConfigProperty#getInferFunc` gets called only in
`ConfigProperty#setDefaultValue(ConfigProperty)` so we should check how the
default is being set at the call site for all such configs with infer function.
--
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]
