lucasbru commented on code in PR #18383:
URL: https://github.com/apache/kafka/pull/18383#discussion_r1901968922
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -741,6 +741,7 @@ public
List<StreamsGroupDescribeResponseData.DescribedGroup> streamsGroupDescrib
describedGroups.add(new
StreamsGroupDescribeResponseData.DescribedGroup()
.setGroupId(groupId)
.setErrorCode(Errors.GROUP_ID_NOT_FOUND.code())
+ .setErrorMessage(exception.getMessage())
Review Comment:
This was actually inconsistent to other group types
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/taskassignor/StickyTaskAssignorTest.java:
##########
@@ -18,6 +18,7 @@
package org.apache.kafka.coordinator.group.taskassignor;
import org.apache.kafka.coordinator.group.GroupCoordinatorConfig;
+
Review Comment:
spotlessApply
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java:
##########
@@ -15673,6 +15676,349 @@ public void testReplayStreamsGroupTopologyTombstone()
{
assertThrows(GroupIdNotFoundException.class, () ->
context.groupMetadataManager.streamsGroup("bar"));
}
+ @Test
+ public void testConsumerGroupHeartbeatOnStreamsGroup() {
Review Comment:
The test are pretty repetitive - like the ones for the other group types. I
considered saving some LOC (by using private helper methods, and parametrized
tests), but kept them at way, since it's going to be easier to refactor if each
test is self-contained.
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java:
##########
@@ -9373,7 +9375,8 @@ public void
testStreamsGroupDescribeBeforeAndAfterCommittingOffset() {
List<StreamsGroupDescribeResponseData.DescribedGroup> actual =
context.groupMetadataManager.streamsGroupDescribe(Collections.singletonList(streamsGroupId),
context.lastCommittedOffset);
StreamsGroupDescribeResponseData.DescribedGroup describedGroup = new
StreamsGroupDescribeResponseData.DescribedGroup()
.setGroupId(streamsGroupId)
- .setErrorCode(Errors.GROUP_ID_NOT_FOUND.code());
+ .setErrorCode(Errors.GROUP_ID_NOT_FOUND.code())
+ .setErrorMessage("Group " + streamsGroupId + " not found.");
Review Comment:
Updates behavior according to the above change to production code.
--
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]
