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 <j...@confluent.io> wrote:
> 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 <j...@confluent.io> wrote:
>
> > 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 <stevenz...@gmail.com>
> wrote:
> >
> > > 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
> > > > > >>
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
