Re: [DISCUSSION] java.io.Closeable in KAFKA-2121

2015-05-01 Thread Jun Rao
If you use KafkaProducer as a Closable, you still need to catch the exception when calling close(), right? So the behavior is different whether you use it as a Producer or a Closable? Thanks, Jun On Thu, Apr 30, 2015 at 6:26 PM, Jay Kreps wrote: > Hey Jun, > > I think the Closable interface is

Re: [DISCUSSION] java.io.Closeable in KAFKA-2121

2015-04-30 Thread Jay Kreps
Hey Jun, I think the Closable interface is what we've used elsewhere and what the rest of the java world uses. I don't think it is too hard for us to add the override in our interface--implementors of the interface don't need to do it. -Jay On Thu, Apr 30, 2015 at 4:02 PM, Jun Rao wrote: > Tha

Re: [DISCUSSION] java.io.Closeable in KAFKA-2121

2015-04-30 Thread Jun Rao
That makes sense. Then, would it be better to have a KafkaClosable interface that doesn't throw exception? This way, we don't need to override close in every implementing class. Thanks, Jun On Wed, Apr 29, 2015 at 10:36 AM, Steven Wu wrote: > Jun, > > we still get the benefit of extending Clos

Re: [DISCUSSION] java.io.Closeable in KAFKA-2121

2015-04-29 Thread Guozhang Wang
I think I agree with Jay / Steven, that we still get the benefit of extending Closeable and overriding close(). Guozhang On Wed, Apr 29, 2015 at 8:36 AM, Steven Wu wrote: > Jun, > > we still get the benefit of extending Closeable. e.g. Utils.closeQuietly() > can take FooSerializer as an argumen

Re: [DISCUSSION] java.io.Closeable in KAFKA-2121

2015-04-29 Thread Steven Wu
Jun, we still get the benefit of extending Closeable. e.g. Utils.closeQuietly() can take FooSerializer as an argument. we can avoid the duplication of boiler-plate code. class FooSerializer implements Serializer { @Override public void close() { // may throw unchecked RuntimeExce

Re: [DISCUSSION] java.io.Closeable in KAFKA-2121

2015-04-29 Thread Jun Rao
If you do this, the code is no longer simple, which defeats the benefit of extending Closeable. We can define our own Closeable that doesn't throw exceptions, but it may be confusing. So, it seems the original code is probably better. Thanks, Jun On Tue, Apr 28, 2015 at 3:11 PM, Steven Wu wrote

Re: [DISCUSSION] java.io.Closeable in KAFKA-2121

2015-04-28 Thread Steven Wu
sorry for the previous empty msg. Jay's idea should work. basically, we override the close method in Serializer interface. public interface Serializer extends Closeable { @Override public void close(); } On Tue, Apr 28, 2015 at 1:10 PM, Steven Wu wrote: > > > On Tue, Apr 28, 2015 at 1:

Re: [DISCUSSION] java.io.Closeable in KAFKA-2121

2015-04-28 Thread Steven Wu
On Tue, Apr 28, 2015 at 1:03 PM, Ewen Cheslack-Postava wrote: > Good point Jay. More specifically we were already implementing without the > checked exception, we'd need to override close() in the Serializer and > Deserializer interfaces and omit the throws clause. That definitely makes > them so

Re: [DISCUSSION] java.io.Closeable in KAFKA-2121

2015-04-28 Thread Ewen Cheslack-Postava
Good point Jay. More specifically we were already implementing without the checked exception, we'd need to override close() in the Serializer and Deserializer interfaces and omit the throws clause. That definitely makes them source compatible. Not sure about binary compatibility, I couldn't find a

Re: [DISCUSSION] java.io.Closeable in KAFKA-2121

2015-04-28 Thread Jay Kreps
Hey guys, You can implement Closable without the checked exception. Having close() methods throw checked exceptions isn't very useful unless there is a way for the caller to recover. In this case there really isn't, right? -Jay On Mon, Apr 27, 2015 at 5:51 PM, Guozhang Wang wrote: > Folks, > >

Re: [DISCUSSION] java.io.Closeable in KAFKA-2121

2015-04-28 Thread Ewen Cheslack-Postava
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

[DISCUSSION] java.io.Closeable in KAFKA-2121

2015-04-27 Thread Guozhang Wang
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 keySerializer = ... Seri