aliehsaeedii commented on code in PR #17981:
URL: https://github.com/apache/kafka/pull/17981#discussion_r1867833556
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupStore.java:
##########
@@ -0,0 +1,307 @@
+/*
Review Comment:
Thanks, @dajac, for the conceptual review.
>I am also tempted by moving all the replay methods to the store
Agree! That makes sense and makes the utests cleaner.
>other methods (e.g. validateDeleteGroup, maybeDeleteGroup,
createGroupTombstoneRecords, etc should not be here in my opinion.
Your opinion is valid David. Since other manager classes have a reference to
`GroupStore`, I assumed such methods could be inside this class to be
accessible by all other manager classes. But I think, later we can have a
helper class and these methods could be defined there.
>The API specific methods should resides outside of the store. e.g.
listGroups
`listGroups`: REMOVED
>The store does not need a reference to a MetadataImage.
I removed the `MetadataImage` and the associated methods from this class. Do
you mean each manager class should have its own `MetadataImage` instance?
--
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]
