mjsax commented on code in PR #23364:
URL: https://github.com/apache/kafka/pull/23364#discussion_r3972058123
##########
streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java:
##########
@@ -1926,6 +1939,7 @@ public Map<String, Object> getMainConsumerConfigs(final
String groupId, final St
final Map<String, Object> mainConsumerProps =
originalsWithPrefix(MAIN_CONSUMER_PREFIX);
checkIfUnexpectedUserSpecifiedClientConfig(mainConsumerProps,
NON_CONFIGURABLE_CONSUMER_DEFAULT_CONFIGS);
consumerProps.putAll(mainConsumerProps);
+ enforceSynchronousBootstrapResolution(consumerProps, "consumer");
Review Comment:
Well yes, but my point is, the new `enforceSynchronousBootstrapResolution`
is a special case impl, while `checkIfUnexpectedUserSpecifiedClientConfig` is a
generic one, so why not re-use the generic one (and extend it as needed)?
We don't need to block this PR on it, to make sure 4.4 can move forward, but
it seems to set us up for some spaghetti-code, if we add a new helper for every
config we want to lock down. `checkIfUnexpectedUserSpecifiedClientConfig` is a
generic one, that should cover all cases in a central place, an it's getting
parametrized (so we only need to add new config we lock down to the
corresponding collection of "locked down" configs).
But as said: also ok to merge this as-is, and cherry-pick to 4.4, to unblock
4.4. But would like to see a follow up PR (trunk only is ok) to clean it up.
--
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]
