mjsax commented on code in PR #23092:
URL: https://github.com/apache/kafka/pull/23092#discussion_r3761081117
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/streams/assignor/AssignmentConfigsImpl.java:
##########
@@ -61,17 +60,24 @@ public static AssignmentConfigsImpl fromMap(Map<String,
String> configs) {
if (configs.isEmpty()) {
return DEFAULT;
}
- // The rack-aware assignment tags are only set when any are
configured, and are joined from values that
- // ConfigDef has already validated to be non-empty and free of
surrounding whitespace.
- String rackAwareAssignmentTags =
configs.get(RACK_AWARE_ASSIGNMENT_TAGS_CONFIG);
+ // The rack-aware assignment tags are only recorded when any are
configured, so an absent value means the
+ // configuration is at its default.
+ String rackAwareAssignmentTags = configs.getOrDefault(
+ RACK_AWARE_ASSIGNMENT_TAGS_CONFIG,
GroupCoordinatorConfig.STREAMS_GROUP_RACK_AWARE_ASSIGNMENT_TAGS_DEFAULT);
return new AssignmentConfigsImpl(
Integer.parseInt(configs.get(NUM_STANDBY_REPLICAS_CONFIG)),
- rackAwareAssignmentTags == null
- ? List.of()
- : List.of(rackAwareAssignmentTags.split(","))
+ parseRackAwareAssignmentTags(rackAwareAssignmentTags)
);
}
+ /**
+ * Parses a recorded rack-aware assignment tags value, the way {@link
org.apache.kafka.common.config.ConfigDef}
+ * parses a {@code LIST} configuration: an empty value is an empty list,
not a list holding an empty string.
+ */
+ private static List<String> parseRackAwareAssignmentTags(String
rackAwareAssignmentTags) {
+ return rackAwareAssignmentTags.isEmpty() ? List.of() :
List.of(rackAwareAssignmentTags.split(","));
Review Comment:
`isEmpty()` checks for length() == 0, right? What about `" "` (or similar).
Do we need to to do rackAwareAssignmentTags.trim().isEmpty()` ?
--
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]
