You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/05/05 12:08:21 UTC
[GitHub] [kafka] lqjack opened a new pull request, #12125: provide the construct interceptor for KafkaProducer and KafkaConsumer
lqjack opened a new pull request, #12125:
URL: https://github.com/apache/kafka/pull/12125
*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*
*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] frosiere commented on a diff in pull request #12125: KAFKA-13864: provide the construct interceptor for KafkaProducer and KafkaConsumer
Posted by GitBox <gi...@apache.org>.
frosiere commented on code in PR #12125:
URL: https://github.com/apache/kafka/pull/12125#discussion_r866287593
##########
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java:
##########
@@ -663,12 +663,38 @@ public KafkaConsumer(Properties properties,
public KafkaConsumer(Map<String, Object> configs,
Deserializer<K> keyDeserializer,
Deserializer<V> valueDeserializer) {
+ this(configs, keyDeserializer, valueDeserializer, null);
+ }
+
+ /**
+ * A consumer is instantiated by providing a set of key-value pairs as configuration, and a key and a value {@link Deserializer}.
+ * <p>
+ * Valid configuration strings are documented at {@link ConsumerConfig}.
+ * <p>
+ * Note: after creating a {@code KafkaConsumer} you must always {@link #close()} it to avoid resource leaks.
+ *
+ * @param configs The consumer configs
+ * @param keyDeserializer The deserializer for key that implements {@link Deserializer}. The configure() method
+ * won't be called in the consumer when the deserializer is passed in directly.
+ * @param valueDeserializer The deserializer for value that implements {@link Deserializer}. The configure() method
+ * 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,
Review Comment:
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: https://github.com/spring-projects/spring-kafka/issues/2244
This approach can work as well.
##########
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java:
##########
@@ -663,12 +663,38 @@ public KafkaConsumer(Properties properties,
public KafkaConsumer(Map<String, Object> configs,
Deserializer<K> keyDeserializer,
Deserializer<V> valueDeserializer) {
+ this(configs, keyDeserializer, valueDeserializer, null);
+ }
+
+ /**
+ * A consumer is instantiated by providing a set of key-value pairs as configuration, and a key and a value {@link Deserializer}.
Review Comment:
the new parameter is not mentioned
##########
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java:
##########
@@ -290,6 +290,27 @@ public KafkaProducer(Map<String, Object> configs, Serializer<K> keySerializer, S
keySerializer, valueSerializer, null, null, null, Time.SYSTEM);
}
+ /**
+ * A producer is instantiated by providing a set of key-value pairs as configuration, a key and a value {@link Serializer}.
Review Comment:
Same issue with the Javadoc of this constructor
##########
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java:
##########
@@ -663,12 +663,38 @@ public KafkaConsumer(Properties properties,
public KafkaConsumer(Map<String, Object> configs,
Deserializer<K> keyDeserializer,
Deserializer<V> valueDeserializer) {
+ this(configs, keyDeserializer, valueDeserializer, null);
+ }
+
+ /**
+ * A consumer is instantiated by providing a set of key-value pairs as configuration, and a key and a value {@link Deserializer}.
+ * <p>
+ * Valid configuration strings are documented at {@link ConsumerConfig}.
+ * <p>
+ * Note: after creating a {@code KafkaConsumer} you must always {@link #close()} it to avoid resource leaks.
+ *
+ * @param configs The consumer configs
+ * @param keyDeserializer The deserializer for key that implements {@link Deserializer}. The configure() method
+ * won't be called in the consumer when the deserializer is passed in directly.
+ * @param valueDeserializer The deserializer for value that implements {@link Deserializer}. The configure() method
+ * 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}.
Review Comment:
Java doc is not correct. The list of interceptors... $link should be replaced by @link
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] frosiere commented on pull request #12125: KAFKA-13864: provide the construct interceptor for KafkaProducer and KafkaConsumer
Posted by GitBox <gi...@apache.org>.
frosiere commented on PR #12125:
URL: https://github.com/apache/kafka/pull/12125#issuecomment-1119022631
Tests and checks are failing.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] frosiere commented on pull request #12125: KAFKA-13864: provide the construct interceptor for KafkaProducer and KafkaConsumer
Posted by GitBox <gi...@apache.org>.
frosiere commented on PR #12125:
URL: https://github.com/apache/kafka/pull/12125#issuecomment-1119923373
@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
Could you please apply changes as described in the KIP?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] lqjack commented on a diff in pull request #12125: KAFKA-13864: provide the construct interceptor for KafkaProducer and KafkaConsumer
Posted by GitBox <gi...@apache.org>.
lqjack commented on code in PR #12125:
URL: https://github.com/apache/kafka/pull/12125#discussion_r875402198
##########
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java:
##########
@@ -663,12 +663,38 @@ public KafkaConsumer(Properties properties,
public KafkaConsumer(Map<String, Object> configs,
Deserializer<K> keyDeserializer,
Deserializer<V> valueDeserializer) {
+ this(configs, keyDeserializer, valueDeserializer, null);
+ }
+
+ /**
+ * A consumer is instantiated by providing a set of key-value pairs as configuration, and a key and a value {@link Deserializer}.
Review Comment:
update the java doc for the param
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] lqjack commented on a diff in pull request #12125: KAFKA-13864: provide the construct interceptor for KafkaProducer and KafkaConsumer
Posted by GitBox <gi...@apache.org>.
lqjack commented on code in PR #12125:
URL: https://github.com/apache/kafka/pull/12125#discussion_r875399898
##########
clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java:
##########
@@ -663,12 +663,38 @@ public KafkaConsumer(Properties properties,
public KafkaConsumer(Map<String, Object> configs,
Deserializer<K> keyDeserializer,
Deserializer<V> valueDeserializer) {
+ this(configs, keyDeserializer, valueDeserializer, null);
+ }
+
+ /**
+ * A consumer is instantiated by providing a set of key-value pairs as configuration, and a key and a value {@link Deserializer}.
+ * <p>
+ * Valid configuration strings are documented at {@link ConsumerConfig}.
+ * <p>
+ * Note: after creating a {@code KafkaConsumer} you must always {@link #close()} it to avoid resource leaks.
+ *
+ * @param configs The consumer configs
+ * @param keyDeserializer The deserializer for key that implements {@link Deserializer}. The configure() method
+ * won't be called in the consumer when the deserializer is passed in directly.
+ * @param valueDeserializer The deserializer for value that implements {@link Deserializer}. The configure() method
+ * 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}.
Review Comment:
fixed
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] frosiere commented on pull request #12125: KAFKA-13864: provide the construct interceptor for KafkaProducer and KafkaConsumer
Posted by GitBox <gi...@apache.org>.
frosiere commented on PR #12125:
URL: https://github.com/apache/kafka/pull/12125#issuecomment-1129661166
As mentioned in the Jira ticket, KIP-832 won't be implemented and will be replaced by KIP-839.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org