ableegoldman commented on a change in pull request #8716:
URL: https://github.com/apache/kafka/pull/8716#discussion_r430778603
##########
File path: streams/src/main/java/org/apache/kafka/streams/StreamsConfig.java
##########
@@ -1148,6 +1148,9 @@ private void verifyMaxInFlightRequestPerConnection(final
Object maxInFlightReque
consumerProps.put(REPLICATION_FACTOR_CONFIG,
getInt(REPLICATION_FACTOR_CONFIG));
consumerProps.put(APPLICATION_SERVER_CONFIG,
getString(APPLICATION_SERVER_CONFIG));
consumerProps.put(NUM_STANDBY_REPLICAS_CONFIG,
getInt(NUM_STANDBY_REPLICAS_CONFIG));
+ consumerProps.put(ACCEPTABLE_RECOVERY_LAG_CONFIG,
getLong(ACCEPTABLE_RECOVERY_LAG_CONFIG));
+ consumerProps.put(MAX_WARMUP_REPLICAS_CONFIG,
getInt(MAX_WARMUP_REPLICAS_CONFIG));
+ consumerProps.put(PROBING_REBALANCE_INTERVAL_MS_CONFIG,
getLong(PROBING_REBALANCE_INTERVAL_MS_CONFIG));
Review comment:
> we automatically pass through any config that isn't registered
I have to say, this seems totally backwards to me. So basically we just
happen to pass in any number of configs that we may or may not need, but will
split out specific configs that we _do_ need unless explicitly told to include
them? I understand the custom configs motivation, but then why not just pass
through all the configs?
What if I wanted to access the value of one of my registered Streams configs
in my plugged-in component? I'd have to add the same value a second time, but
with an unregistered config name. Huh?
##########
File path:
streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java
##########
@@ -347,22 +348,28 @@ public AssignmentListener assignmentListener() {
public final long probingRebalanceIntervalMs;
Review comment:
I think this comment got lost in a discussion thread, but can we add a
note to AssignorConfiguration pointing out that any Streams configs added here
will need to be explicitly passed through? It seems like it's too easy to fall
into this same trap again
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
