-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[FLINK-35700][cli] Loosen CDC pipeline options validation #3435
Conversation
String.format("Unknown configuration key `%s`", key)); | ||
} | ||
}); | ||
|
||
options.forEach( | ||
option -> { | ||
if (!configuration.getOptional(option).isPresent() |
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.
To be honest, I also don't understand why unspecified configuration without default value is error?
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.
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.
Since required / optional configuration in pipeline is vague (and undocumented), I've removed all pipeline-level validations.
+1 to follow Flink Conf‘s validation style |
LGTM! |
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.
+1
This closes FLINK-35700.
FLINK-35121 adds pipeline configuration validation, rejecting any unknown options, which turns out to be too strict, and it's not possible to create customized configuration extensions. Also, Flink doesn't reject unknown entries in flink-conf / config.yaml, just silently ignores them. It might be better for CDC to follow such behavior.