pvary commented on code in PR #8553:
URL: https://github.com/apache/iceberg/pull/8553#discussion_r1323970769
##########
flink/v1.17/flink/src/main/java/org/apache/iceberg/flink/source/IcebergSource.java:
##########
@@ -237,6 +248,9 @@ public Builder<T> table(Table newTable) {
}
public Builder<T> assignerFactory(SplitAssignerFactory assignerFactory) {
+ Preconditions.checkArgument(
Review Comment:
This check is here to specifically prevent the user providing conflicting
settings, since when the `eventTimeExtractor` is set, we will automatically set
the `assinerFactory`. So checking them later, especially when we already
changed them would be confusing.
Also, having a feedback immediately about the problematic settings is much
better than somewhere later in the code.
This is more opinionated, but I think the final `checkRequired` check is
less about the user parametrization, and more about making sure that the code
set everything which is needed for the source to run. So in my opinion it is
more like ensuring that the Iceberg code is correct, and less about the user
provided parameters.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
