I suggested the change to Closeable mainly to avoid some verbosity and redundancy in the code since we have to call close() on multiple components and without a common interface, the error handling code is repeated. Using Closeable does make the code cleaner and more maintainable, but the issue was pretty quickly identified, which suggests there might be more people calling close() on the serializer.
I think we should probably revert the use of Closeable and just use the more verbose version. I'm definitely in favor of being pretty strict about compatibility, and the readability/maintainability cost here isn't awful. Given some other thoughts about system and protocol compatibility tests like Gwen suggests in KAFKA-2003, it'd probably also be worth having a few sample client applications that we can compile against version X and run against version Y to verify API compatibility. Even just a few different examples from different developers could give us reasonable API coverage. On Mon, Apr 27, 2015 at 5:51 PM, Guozhang Wang <wangg...@gmail.com> wrote: > Folks, > > In a recent commit I made regarding KAFKA-2121, there is an omitted API > change which makes Serializer / Deserializer extending from Closeable, > whose close() call could throw IOException by declaration. Hence now some > scenario like: > > --------------------- > > Serializer<T> keySerializer = ... > Serializer<T> valueSerializer = ... > KafkaProducer producer = new KafkaProducer(config, keySerializer, > valueSerializer) > // ... > keySerializer.close() > valueSerializer.close() > > --------------------- > > will need to capture IOException now. > > Want to bring this up for people's attention, and you opinion on whether we > should revert this change? > > -- Guozhang > -- Thanks, Ewen
