bbejeck commented on code in PR #13527:
URL: https://github.com/apache/kafka/pull/13527#discussion_r1167075556


##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/assignment/AssignorConfiguration.java:
##########
@@ -261,41 +261,29 @@ public interface AssignmentListener {
         void onAssignmentComplete(final boolean stable);
     }
 
-    /**
-     * NOTE: any StreamsConfig you add here MUST be passed in to the consumer 
via

Review Comment:
   This doesn't seem specific to KIP-878 - should it remain?



##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignorTest.java:
##########
@@ -71,7 +67,23 @@
 import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.fail;
 
-public class HighAvailabilityTaskAssignorTest {    
+public class HighAvailabilityTaskAssignorTest {
+    private final AssignmentConfigs configWithoutStandbys = new 
AssignmentConfigs(
+        /*acceptableRecoveryLag*/ 100L,
+        /*maxWarmupReplicas*/ 2,
+        /*numStandbyReplicas*/ 0,
+        /*probingRebalanceIntervalMs*/ 60 * 1000L,
+        /*rackAwareAssignmentTags*/ EMPTY_RACK_AWARE_ASSIGNMENT_TAGS
+    );
+
+    private final AssignmentConfigs configWithStandbys = new AssignmentConfigs(

Review Comment:
   As a side note, I think these two inner classes would benefit from using the 
`Builder` pattern. While the comments are useful here, it's hard to discern 
what the different parameters are when looking at the individual tests.  I'm 
not suggesting this be done in this PR but in a follow-up one if you agree.



-- 
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