lianetm commented on code in PR #19024:
URL: https://github.com/apache/kafka/pull/19024#discussion_r1970153512


##########
clients/src/test/java/org/apache/kafka/clients/consumer/internals/ConsumerHeartbeatRequestManagerTest.java:
##########
@@ -1029,7 +1029,8 @@ private static Collection<Arguments> errorProvider() {
             Arguments.of(Errors.UNSUPPORTED_VERSION, true),
             Arguments.of(Errors.UNRELEASED_INSTANCE_ID, true),
             Arguments.of(Errors.FENCED_INSTANCE_ID, true),
-            Arguments.of(Errors.GROUP_MAX_SIZE_REACHED, true));
+            Arguments.of(Errors.GROUP_MAX_SIZE_REACHED, true),
+            Arguments.of(Errors.TOPIC_AUTHORIZATION_FAILED, true));

Review Comment:
   > the ShareHeartbeatRequestManagerTest, yes they are identical, except there 
is one check missing
   
   Exactly, that shows how the Share test has already fallen behind, it is 
redundant and we're not maintaining it. We should review and remove it imo (but 
can be done separately, I just noticed it here) 
   
   >if we decide to handle heartbeat responses differently for regular 
consumers and share consumers, it will be better to have 2 different
   
   yes, but better to add it when needed (and not have a duplicate now that we 
either maintain or leave behind)
   
   I will file a jira and we can consider the improvement separately. 
   -- update
   https://issues.apache.org/jira/browse/KAFKA-18862



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