yihua commented on code in PR #7869:
URL: https://github.com/apache/hudi/pull/7869#discussion_r1128343130
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -207,6 +207,7 @@ public class HoodieWriteConfig extends HoodieConfig {
public static final ConfigProperty<String> BASE_PATH = ConfigProperty
.key("hoodie.base.path")
.noDefaultValue()
+ .makeEssential(true)
Review Comment:
IMO, by defualt, the `essential` property should be false, meaning that the
config has to be marked "essential" explicitly, enforcing us to think twice
when introducing a new essential config. For this API, we can remove the
argument, i.e., `.makeEssential()` instead of `.makeEssential(true)`.
##########
hudi-common/src/main/java/org/apache/hudi/common/config/ConfigProperty.java:
##########
@@ -127,34 +130,42 @@ public List<String> getAlternatives() {
return Arrays.asList(alternatives);
}
+ public boolean isEssential() {
+ return essential;
+ }
+
public ConfigProperty<T> withDocumentation(String doc) {
Objects.requireNonNull(doc);
- return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
sinceVersion, deprecatedVersion, inferFunction, validValues, alternatives);
+ return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
sinceVersion, deprecatedVersion, inferFunction, validValues, essential,
alternatives);
}
public ConfigProperty<T> withValidValues(String... validValues) {
Objects.requireNonNull(validValues);
- return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
sinceVersion, deprecatedVersion, inferFunction, new
HashSet<>(Arrays.asList(validValues)), alternatives);
+ return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
sinceVersion, deprecatedVersion, inferFunction, new
HashSet<>(Arrays.asList(validValues)), essential, alternatives);
}
public ConfigProperty<T> withAlternatives(String... alternatives) {
Objects.requireNonNull(alternatives);
- return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
sinceVersion, deprecatedVersion, inferFunction, validValues, alternatives);
+ return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
sinceVersion, deprecatedVersion, inferFunction, validValues, essential,
alternatives);
}
public ConfigProperty<T> sinceVersion(String sinceVersion) {
Objects.requireNonNull(sinceVersion);
- return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
Option.of(sinceVersion), deprecatedVersion, inferFunction, validValues,
alternatives);
+ return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
Option.of(sinceVersion), deprecatedVersion, inferFunction, validValues,
essential, alternatives);
}
public ConfigProperty<T> deprecatedAfter(String deprecatedVersion) {
Objects.requireNonNull(deprecatedVersion);
- return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
sinceVersion, Option.of(deprecatedVersion), inferFunction, validValues,
alternatives);
+ return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
sinceVersion, Option.of(deprecatedVersion), inferFunction, validValues,
essential, alternatives);
}
public ConfigProperty<T> withInferFunction(Function<HoodieConfig, Option<T>>
inferFunction) {
Objects.requireNonNull(inferFunction);
- return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
sinceVersion, deprecatedVersion, Option.of(inferFunction), validValues,
alternatives);
+ return new ConfigProperty<>(key, defaultValue, docOnDefaultValue, doc,
sinceVersion, deprecatedVersion, Option.of(inferFunction), validValues,
essential, alternatives);
+ }
+
+ public ConfigProperty<T> makeEssential(boolean isEssential) {
Review Comment:
As mentioned above, change this to `makeEssential()` without taking any
argument.
##########
hudi-common/src/main/java/org/apache/hudi/common/config/ConfigProperty.java:
##########
@@ -171,9 +182,9 @@ public static PropertyBuilder key(String key) {
@Override
public String toString() {
return String.format(
- "Key: '%s' , default: %s description: %s since version: %s deprecated
after: %s)",
+ "Key: '%s' , default: %s description: %s since version: %s deprecated
after: %s , is essential: %s)",
Review Comment:
nit: add this before `description: %s` since the description can be long?
##########
hudi-common/src/main/java/org/apache/hudi/common/table/HoodieTableConfig.java:
##########
@@ -91,11 +91,13 @@ public class HoodieTableConfig extends HoodieConfig {
public static final ConfigProperty<String> NAME = ConfigProperty
.key(HOODIE_TABLE_NAME_KEY)
.noDefaultValue()
+ .makeEssential(true)
.withDocumentation("Table name that will be used for registering with
Hive. Needs to be same across runs.");
public static final ConfigProperty<HoodieTableType> TYPE = ConfigProperty
.key("hoodie.table.type")
.defaultValue(HoodieTableType.COPY_ON_WRITE)
+ .makeEssential(true)
Review Comment:
Remove these since these are not supposed to be tweaked by the user?
--
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]
