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