dajac commented on code in PR #23896:
URL: https://github.com/apache/kafka/pull/23896#discussion_r4358195363
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/GroupSpecImpl.java:
##########
@@ -22,19 +22,72 @@
import org.apache.kafka.coordinator.group.api.assignor.MemberAssignment;
import org.apache.kafka.coordinator.group.api.assignor.MemberSubscription;
import org.apache.kafka.coordinator.group.api.assignor.SubscriptionType;
+import org.apache.kafka.coordinator.group.modern.share.ShareGroup.InitMapValue;
import java.util.BitSet;
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
-import java.util.Optional;
import java.util.Set;
/**
* The assignment specification for a modern group.
*/
public class GroupSpecImpl implements GroupSpec {
+ /**
+ * The assignment specification for a share group.
+ */
+ public static class ShareGroupSpecImpl extends GroupSpecImpl {
Review Comment:
We usually use this pattern when the top class is abstract. Calling new
`GroupSpecImpl.ShareGroupSpecImpl()` feels a bit weird. How about putting
`ShareGroupSpecImpl` in its own file and renaming `GroupSpecImpl` to
`ConsumerGroupSpecImpl`?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/GroupSpecBuilder.java:
##########
@@ -242,22 +269,14 @@ public GroupSpec build() {
))
);
- Optional<Map<Uuid, Set<Integer>>> topicAssignablePartitionsMap =
this.topicAssignablePartitionsMap;
if (assignorOffload) {
Review Comment:
I wonder whether we should just remove `assignorOffload` now. Dead code
looks weird.
--
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]
