chia7712 commented on code in PR #16899:
URL: https://github.com/apache/kafka/pull/16899#discussion_r1743594446


##########
clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java:
##########
@@ -727,6 +731,20 @@ private void checkGroupRemoteAssignor() {
         }
     }
 
+    @SuppressWarnings("unchecked")
+    private void checkPartitionAssigmentStrategy() {
+        List<Object> assignmentStrategies = (List<Object>) 
get(PARTITION_ASSIGNMENT_STRATEGY_CONFIG);
+        if 
(getString(GROUP_PROTOCOL_CONFIG).equalsIgnoreCase(GroupProtocol.CONSUMER.name()))
 {
+            if (PARTITION_ASSIGNOR_DEFAULT_VALUE.size() != 
assignmentStrategies.size() ||
+                    !PARTITION_ASSIGNOR_DEFAULT_VALUE.stream()
+                            .allMatch(clz -> assignmentStrategies.stream()
+                                    .anyMatch(obj -> clz.equals(obj) || 
clz.getName().equals(obj)))
+            ) {
+                throw new ConfigException(PARTITION_ASSIGNMENT_STRATEGY_CONFIG 
+ " cannot be set when " + GROUP_PROTOCOL_CONFIG + "=" + 
GroupProtocol.CONSUMER.name());
+            }
+        }
+    }
+

Review Comment:
   +1 to @kirktrue @lianetm that checking the user-defined values makes sense 
to me.
   
   For another, `ConsumerConfig`'s constructor will log all configs, so it 
would be nice to set the config `partition.assignment.strategy` to null when 
the protocol is `CONSUMER`.  This behavior is similar to 
`maybeOverrideEnableAutoCommit` which can avoid printing "weird" consumer 
configs in log file.
   
   



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to