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 RuntimeException
}
}
final class Utils {
public static void closeQuietly(Closeable c, String name,
AtomicReference<Throwable> firstException) {
if (c != null) {
try {
c.close();
} catch (Throwable t) {
firstException.compareAndSet(null, t);
log.error("Failed to close " + name, t);
}
}
}
On Wed, Apr 29, 2015 at 6:51 AM, Jun Rao <j...@confluent.io> wrote:
> 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 <stevenz...@gmail.com> wrote:
>
> > sorry for the previous empty msg.
> >
> > Jay's idea should work. basically, we override the close method in
> > Serializer interface.
> >
> > public interface Serializer<T> extends Closeable {
> > @Override
> > public void close();
> > }
> >
> > On Tue, Apr 28, 2015 at 1:10 PM, Steven Wu <stevenz...@gmail.com> wrote:
> >
> > >
> > >
> > > On Tue, Apr 28, 2015 at 1:03 PM, Ewen Cheslack-Postava <
> > e...@confluent.io>
> > > 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 source compatible. Not sure about binary compatibility, I
> couldn't
> > >> find a quick answer but I think it's probably still compatible.
> > >>
> > >> -Ewen
> > >>
> > >> On Tue, Apr 28, 2015 at 12:30 PM, Jay Kreps <jay.kr...@gmail.com>
> > wrote:
> > >>
> > >> > 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 <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
> > >>
> > >
> > >
> >
>
