Github user srdo commented on a diff in the pull request:
https://github.com/apache/storm/pull/2428#discussion_r155916418
--- Diff:
external/storm-kafka-client/src/main/java/org/apache/storm/kafka/spout/KafkaSpoutConfig.java
---
@@ -359,7 +356,7 @@ private Builder(Builder<?, ?> builder,
SerializableDeserializer<K> keyDes, Class
*/
@Deprecated
public <NK> Builder<NK, V> setKey(Class<? extends
Deserializer<NK>> clazz) {
--- End diff --
Yes, the reasons you mention are the reasons these methods return new
builders.
I agree that there is no reason for the fields to be final.
The snippet breaks the ability to change the key/value deserializer types.
The existing code allows you to do e.g.
```
@Test
public void test() {
KafkaSpoutConfig<String, byte[]> conf =
//Use default <String, String> Builder
KafkaSpoutConfig.builder("localhost:1234", "topic")
//Change to byte array value deserializer
.setValue(ByteArrayDeserializer.class)
.build();
}
```
which now fails to compile because the new type bound on setKey/setValue
prevents changing from `Deserializer<String>` to `Deserializer<byte[]>`.
You're right that the existing API is somewhat confusing, but it's getting
removed in 2.0.0 and is deprecated here. I'd rather not add breaking changes to
it if we can avoid it, because if we were going to break the API in a minor
version I think we should just have removed the deprecated methods entirely.
---
