-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add EnumConfParser to SparkConfParser #10311
Conversation
cc @aokolnychyi |
@@ -197,6 +201,34 @@ private Duration toDuration(String time) { | |||
} | |||
} | |||
|
|||
class EnumConfParser<T extends Enum<T>> extends ConfParser<EnumConfParser<T>, T> { | |||
private T defaultValue; | |||
private final Function<String, T> toEnum; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: We usually have final vars before mutable. Can we switch the order of these two?
@@ -70,6 +70,10 @@ public DurationConfParser durationConf() { | |||
return new DurationConfParser(); | |||
} | |||
|
|||
public <T extends Enum<T>> EnumConfParser<T> enumConf(Function<String, T> func) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: What about func
-> toEnum
?
.enumConf(PlanningMode::fromName) | ||
.sessionConf(SparkSQLProperties.DATA_PLANNING_MODE) | ||
.tableProperty(TableProperties.DATA_PLANNING_MODE) | ||
.defaultValue(PlanningMode.fromName(TableProperties.PLANNING_MODE_DEFAULT)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also offer defaultValue(String value)
in addition to defaultValue(T value)
and call toEnum(value)
internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, pending tests.
Thanks @aokolnychyi @nastra |
Add
EnumConfParser
toSparkConfParser
to parse Enum type properties