squah-confluent commented on code in PR #23092:
URL: https://github.com/apache/kafka/pull/23092#discussion_r3729027720
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/streams/TargetAssignmentBuilder.java:
##########
@@ -114,7 +116,22 @@ public TargetAssignmentBuilder(
this.groupId = Objects.requireNonNull(groupId);
this.groupEpoch = groupEpoch;
this.assignor = Objects.requireNonNull(assignor);
- this.assignmentConfigs = Objects.requireNonNull(assignmentConfigs);
+ this.assignmentConfigs =
toAssignmentConfigs(Objects.requireNonNull(assignmentConfigs));
+ }
+
+ /**
+ * Converts the raw assignment configs computed for the group into the
typed configs passed to the assignor.
+ */
+ private static AssignmentConfigs toAssignmentConfigs(Map<String, String>
assignmentConfigs) {
Review Comment:
A better place for this method could be as a constructor or static method on
`AssignmentConfigsImpl`.
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/streams/assignor/MockAssignorTest.java:
##########
@@ -54,7 +53,7 @@ public void testZeroMembers() {
TaskAssignorException ex = assertThrows(TaskAssignorException.class,
() -> assignor.assign(
new GroupSpecImpl(
Map.of(),
- new HashMap<>()
+ new AssignmentConfigsImpl(0, List.of())
Review Comment:
It's going to be really annoying to update all these tests when we add new
assignment configs.
We could consider declaring `AssignmentConfigsImpl.DEFAULT` or adding a
constructor `AssignmentConfigsImpl(Map<String, String>)`.
--
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]
