TaiJuWu commented on code in PR #18850:
URL: https://github.com/apache/kafka/pull/18850#discussion_r1953124219
##########
clients/src/main/java/org/apache/kafka/clients/consumer/GroupProtocol.java:
##########
@@ -23,7 +23,10 @@ public enum GroupProtocol {
CLASSIC("CLASSIC"),
/** Consumer group protocol */
- CONSUMER("CONSUMER");
+ CONSUMER("CONSUMER"),
+
+ /** Share group protocol */
+ SHARE("SHARE");
Review Comment:
Hi @AndrewJSchofield , I meet some issue about this PR and I would like to
get some feedback from you.
The background I know, shareConsumer start from `KafkaShareConsumer` and
regular KafkaConsumer start from `KafkaConsumer`, both of them utilize
`ConsumerConfig` to check and set configuration.
In current design, we can't distinguish the `ConsumerConfig` is used by
`KafkaConsumer` or `KafkaShareConsumer`
Here is my initial thought.
## Method 1
Keep current design (this PR) , we add `Share` to `GroupProtocol` and create
a specific method for ShareConsumer like
`ConsumerConfig.appendDeserializerToConfigForShare`.
In this method, we force to set `GroupProtocol` as `SHARE`.
Pros: we can leverage old class and user does not need to change their code.
Cons: we need adding `GroupProtocol` to `SHARE` and it maybe set by user, it
is a potential issue.
## Method 2
Create a new config (`ShareConsumerConfig`) which is used for
`KafkaShareConsumer` and it can inherit or split from `ConsumerConfig`, we can
do all check within this new class.
Pros: More cleaner since `KafkaConsumer` is very different with
`KafkaShareConsumer`
Cons: User need to change their code and we need to modify KIP.
In general, I like to method 2. Which one do you prefer or do you have any
other suggestions?
--
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