lianetm commented on code in PR #18649:
URL: https://github.com/apache/kafka/pull/18649#discussion_r1923881452
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -878,22 +877,22 @@ ShareGroup getOrMaybeCreatePersistedShareGroup(
Group group = groups.get(groupId);
if (group == null && !createIfNotExists) {
- throw new IllegalStateException(String.format("Share group %s not
found.", groupId));
+ throw new GroupIdNotFoundException(String.format("Share group %s
not found.", groupId));
}
if (group == null) {
ShareGroup shareGroup = new ShareGroup(snapshotRegistry, groupId);
groups.put(groupId, shareGroup);
return shareGroup;
+ } else {
+ if (group.type() == SHARE) {
+ return (ShareGroup) group;
+ } else {
+ // We don't support upgrading/downgrading between protocols at
the moment so
+ // we throw an exception if a group exists with the wrong type.
+ throw new IllegalStateException(String.format("Group %s is not
a share group.", groupId));
Review Comment:
just for my understanding, why is it that we are throwing IllegalState here
instead of `GroupIdNotFoundException` ? seemed to me we were aligning to
throwing GroupIdNotFound with the change above on ln 880, and it did make sense
to me to align also with the behaviour for Consumer groups, that throws
GroupIdNotFound in these scenarios.
--
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]
