GH-3254: Group protocol in ConsumerProperties #3255
Conversation
Fixes: spring-projects#3254 * Add group.protocol property to ConsumerProperties
|
|
||
| /** | ||
| * Set the container group protocol. | ||
| * |
There was a problem hiding this comment.
No blank lines in method Javadocs.
The @since is supposed to be there on the public API.
And why is this property is not set on the target KafkaConsumer in this PR?
There was a problem hiding this comment.
In this PR, we are not really using this property directly - but rather giving an opportunity to set it via the ConsumerProperties. This property needs to be eventually exposed from Spring Boot auto-configuration, so this PR is sort of the ground work. As this new consumer-group evolves, I would imagine that we may need to make more changes in the framework. At this point, we simply pass it down to the KafkaConsumer as it is if it is set. Otherwise, the KafkaConumser uses the defaults. https://github.com/spring-projects/spring-kafka/blob/main/spring-kafka/src/main/java/org/springframework/kafka/listener/KafkaMessageListenerContainer.java#L862
Do you still see a need to set this somewhere in the framework?
| * Container group protocol that will be set on the Kafka Consumer. | ||
| * Default to classic consumer protocol. | ||
| */ | ||
| private String groupProtocol; |
There was a problem hiding this comment.
Isn't there some enum to rely on?
Or we might introduce one as an inner in this ConsumerProperties class...
|
Closing the PR after further discussing it internally. We don't really need this at this point. We can use the same property as below: |
Fixes: #3254