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 2020/07/13 09:52:39 UTC
[GitHub] [kafka] chia7712 opened a new pull request #9013: KAFKA-10044 Deprecate ConsumerConfig#addDeserializerToConfig and Prod…
chia7712 opened a new pull request #9013:
URL: https://github.com/apache/kafka/pull/9013
issue: https://issues.apache.org/jira/browse/KAFKA-10044
### 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] chia7712 commented on pull request #9013: KAFKA-10044 Deprecate ConsumerConfig#addDeserializerToConfig and Prod…
Posted by GitBox <gi...@apache.org>.
chia7712 commented on pull request #9013:
URL: https://github.com/apache/kafka/pull/9013#issuecomment-657762742
@abbccdda thanks for reviews! I have addressed your comment. pleas take a look :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] abbccdda commented on a change in pull request #9013: KAFKA-10044 Deprecate ConsumerConfig#addDeserializerToConfig and Prod…
Posted by GitBox <gi...@apache.org>.
abbccdda commented on a change in pull request #9013:
URL: https://github.com/apache/kafka/pull/9013#discussion_r453913021
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
##########
@@ -492,8 +492,19 @@ private static String parseAcks(String acksString) {
}
}
+ /**
+ * @deprecated Since 2.7.0. This will be removed in a future major release.
+ */
+ @Deprecated
public static Map<String, Object> addSerializerToConfig(Map<String, Object> configs,
Serializer<?> keySerializer, Serializer<?> valueSerializer) {
+ return appendSerializerToConfig(configs, keySerializer, valueSerializer);
+ }
+
+ static Map<String, Object> appendSerializerToConfig(
+ Map<String, Object> configs,
Review comment:
nit: I don't feel strong about the style here, but maybe we should consider align with other functions which has the first parameter on the same line with function name.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] abbccdda merged pull request #9013: KAFKA-10044 Deprecate ConsumerConfig#addDeserializerToConfig and Prod…
Posted by GitBox <gi...@apache.org>.
abbccdda merged pull request #9013:
URL: https://github.com/apache/kafka/pull/9013
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] abbccdda commented on a change in pull request #9013: KAFKA-10044 Deprecate ConsumerConfig#addDeserializerToConfig and Prod…
Posted by GitBox <gi...@apache.org>.
abbccdda commented on a change in pull request #9013:
URL: https://github.com/apache/kafka/pull/9013#discussion_r453749772
##########
File path: clients/src/main/java/org/apache/kafka/clients/producer/ProducerConfig.java
##########
@@ -492,8 +492,17 @@ private static String parseAcks(String acksString) {
}
}
+ /**
+ * @deprecated Since 2.7.0. This will be removed in a future major release.
+ */
+ @Deprecated
public static Map<String, Object> addSerializerToConfig(Map<String, Object> configs,
Serializer<?> keySerializer, Serializer<?> valueSerializer) {
+ return appendSerializerToConfig(configs, keySerializer, valueSerializer);
+ }
+
+ static Map<String, Object> appendSerializerToConfig(Map<String, Object> configs,
Review comment:
Same here, one parameter per line and align
##########
File path: clients/src/main/java/org/apache/kafka/clients/consumer/ConsumerConfig.java
##########
@@ -579,9 +579,19 @@ private void maybeOverrideClientId(Map<String, Object> configs) {
}
}
+ /**
+ * @deprecated Since 2.7.0. This will be removed in a future major release.
+ */
+ @Deprecated
public static Map<String, Object> addDeserializerToConfig(Map<String, Object> configs,
Deserializer<?> keyDeserializer,
Deserializer<?> valueDeserializer) {
+ return appendDeserializerToConfig(configs, keyDeserializer, valueDeserializer);
+ }
+
+ static Map<String, Object> appendDeserializerToConfig(Map<String, Object> configs,
Review comment:
align parameters
##########
File path: clients/src/test/java/org/apache/kafka/clients/consumer/ConsumerConfigTest.java
##########
@@ -100,24 +101,24 @@ public void testDeserializerToMapConfig() {
Map<String, Object> configs = new HashMap<>();
configs.put(ConsumerConfig.KEY_DESERIALIZER_CLASS_CONFIG, keyDeserializerClass);
configs.put(ConsumerConfig.VALUE_DESERIALIZER_CLASS_CONFIG, valueDeserializerClass);
- Map<String, Object> newConfigs = ConsumerConfig.addDeserializerToConfig(configs, null, null);
+ Map<String, Object> newConfigs = ConsumerConfig.appendDeserializerToConfig(configs, null, null);
Review comment:
nit: change the test name to `testAppendDeserializerToConfig`
##########
File path: clients/src/test/java/org/apache/kafka/clients/producer/ProducerConfigTest.java
##########
@@ -68,24 +69,24 @@ public void testSerializerToMapConfig() {
Map<String, Object> configs = new HashMap<>();
configs.put(ProducerConfig.KEY_SERIALIZER_CLASS_CONFIG, keySerializerClass);
configs.put(ProducerConfig.VALUE_SERIALIZER_CLASS_CONFIG, valueSerializerClass);
- Map<String, Object> newConfigs = ProducerConfig.addSerializerToConfig(configs, null, null);
+ Map<String, Object> newConfigs = ProducerConfig.appendSerializerToConfig(configs, null, null);
Review comment:
Similar here, change test name to match function.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org