Spark 3.5: Add validation to SparkConfParser #10315
Conversation
| return self(); | ||
| } | ||
|
|
||
| protected T parseAndCheck(Function<String, T> conversion, T defaultValue) { |
There was a problem hiding this comment.
Is it possible for ProcedureInput to utilize this method as well?
There was a problem hiding this comment.
This one depends on some state, I'll have to take a look ProcedureInput.
aokolnychyi
force-pushed
the
spark-conf-parsing-validation
branch
from
f9d49e0
to
c774a63
Compare
| } | ||
|
|
||
| protected T parseOptionalAndCheck(Function<String, T> conversion, T defaultValue) { | ||
| T value = parse(conversion, defaultValue); |
There was a problem hiding this comment.
Why do we need a new method? In the existing parse() methods, if check != null can be the case to trigger the check?
There was a problem hiding this comment.
You mean what's the benefit of having parseOptionalAndCheck if we can simply call check in children?
There was a problem hiding this comment.
Oh I meant, why not just add in existing base ConfParser.parse(Function<String, T> conversion, T defaultValue) method
if (check != null) {
check(value);
}
| } | ||
| } | ||
|
|
||
| abstract class ConfParser<ThisT, T> { | ||
| private final List<String> optionNames = Lists.newArrayList(); | ||
| private String sessionConfName; | ||
| private String tablePropertyName; | ||
| private Predicate<T> check; |
There was a problem hiding this comment.
Maybe validator or checker is a better name.
|
|
||
| protected T parseOptionalAndCheck(Function<String, T> conversion, T defaultValue) { | ||
| T value = parse(conversion, defaultValue); | ||
| check(value); |
There was a problem hiding this comment.
Should we skip the check if this value is null for optional config?
This PR adds a way to validate parsed configs in
SparkConfParser. It is intended for the following use: