Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-13864: provide the construct interceptor for KafkaProducer and KafkaConsumer #12125

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

lqjack
Copy link
Contributor

@lqjack lqjack commented May 5, 2022

*Provide the constructor for producer and consumer to configure the interceptor.

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@cadonna cadonna changed the title provide the construct interceptor for KafkaProducer and KafkaConsumer KAFKA-13864: provide the construct interceptor for KafkaProducer and KafkaConsumer May 5, 2022
@cadonna cadonna added the kip Requires or implements a KIP label May 5, 2022
* won't be called in the consumer when the deserializer is passed in directly.
* @param interceptors The list interceptors for consumer that implements {$link ConsumerInterceptor}.
*/
public KafkaConsumer(Map<String, Object> configs,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid touching to the existing and offer more flexibility, I would have created a new constructor with a ProducerConfig to allow overriding the method ProducerConfig#getConfiguredInstances to complete the list of interceptors with instances or any other type of instances such as the MetricsReporter of the partitioner. See the Spring issue for a concrete example: spring-projects/spring-kafka#2244
This approach can work as well.

@frosiere
Copy link

frosiere commented May 5, 2022

Tests and checks are failing.

@frosiere
Copy link

frosiere commented May 6, 2022

@lqjack , based on the discussion thread, the KIP has been updated as follow
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211882578#KIP832:Allowcreatingaproducer/consumerusingaproducer/consumerconfig-ProposedChanges

Do you expect to update the PR to follow what is described in the KIP or do you prefer I do it?
To me, both solutions are fine, just let me know.

@frosiere
Copy link

frosiere commented May 18, 2022

As mentioned in the Jira ticket, KIP-832 won't be implemented and will be replaced by KIP-839.
Due to this, I would suggest to close this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kip Requires or implements a KIP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants