You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Yishun Guan <gy...@gmail.com> on 2018/09/26 23:44:31 UTC
[DISCUSSION] KIP-376: Implement AutoClosable on appropriate classes
that has close()
Hi All,
Here is a trivial KIP:
https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
Suggestions are welcome.
Thanks,
Yishun
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate
classes that has close()
Posted by Yishun Guan <gy...@gmail.com>.
Hi Bill,
I see, i just updated the KIP with all the changes. And your second
concern make sense - if that's what everyone think, we can start with
changing KafkaStream first.
Thanks,
Yishun
On Wed, Sep 26, 2018 at 5:18 PM Bill Bejeck <bb...@gmail.com> wrote:
>
> Hi Yishun,
>
> Thanks for the KIP, seems like a useful addition.
>
> Just a couple of minor comments.
>
> Can you list all the changes in the list under "Public Interfaces" in the
> "Proposed Changes" section that way it's clear what's changing? I realize
> they all will be very similar, but it's better to be explicit with the
> proposed changes in a KIP
>
> Also, you are proposing changes across several components and I'm not sure
> if that is possible in a single KIP, but I could very well be wrong on this
> one, so we'll see what others say.
>
> Thanks,
> Bill
>
> On Wed, Sep 26, 2018 at 7:44 PM Yishun Guan <gy...@gmail.com> wrote:
>
> > Hi All,
> >
> > Here is a trivial KIP:
> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
> >
> > Suggestions are welcome.
> >
> > Thanks,
> > Yishun
> >
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate
classes that has close()
Posted by Bill Bejeck <bb...@gmail.com>.
Hi Yishun,
Thanks for the KIP, seems like a useful addition.
Just a couple of minor comments.
Can you list all the changes in the list under "Public Interfaces" in the
"Proposed Changes" section that way it's clear what's changing? I realize
they all will be very similar, but it's better to be explicit with the
proposed changes in a KIP
Also, you are proposing changes across several components and I'm not sure
if that is possible in a single KIP, but I could very well be wrong on this
one, so we'll see what others say.
Thanks,
Bill
On Wed, Sep 26, 2018 at 7:44 PM Yishun Guan <gy...@gmail.com> wrote:
> Hi All,
>
> Here is a trivial KIP:
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
>
> Suggestions are welcome.
>
> Thanks,
> Yishun
>
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate
classes that has close()
Posted by Yishun Guan <gy...@gmail.com>.
Hi Matthias, thank you for pointing this out! I have changed the KIP
to 'RecordCollector extends AutoCloseable`.
As your first concern regarding incompatibility, can you explain why
this is a breaking change? Although that `AutoClosable#close()`
declares `throws
Exception, I think the overridden 'close()' doesn't have to throw an
exception (either throw a subclass of the parent exception, or not
throw one at all is fine, I could be totally wrong.)
Chia-Ping, I am not quite sure I understand your idea. With the same
idea above, `AutoClosable#close()` is board enough to cover unchecked
and checked exception, right? Could you elaborate?
Thanks,
Yishun
On Sun, Sep 30, 2018 at 1:20 PM Matthias J. Sax <ma...@confluent.io> wrote:
>
> Closeable is part of `java.io` while AutoClosable is part of
> `java.lang`. Thus, the second one is more generic. Also, JavaDoc points
> out that `Closable#close()` must be idempotent while
> `AutoClosable#close()` can have side effects.
>
> Thus, I am not sure atm which one suits better.
>
> However, it's a good hint, that `AutoClosable#close()` declares `throws
> Exception` and thus, it seems to be a backward incompatible change.
> Hence, I am not sure if we can actually move forward easily with this KIP.
>
> Nit: `RecordCollectorImpl` is an internal class that implements
> `RecordCollector` -- should `RecordCollector extends AutoCloseable`?
>
>
> -Matthias
>
>
> On 9/27/18 7:46 PM, Chia-Ping Tsai wrote:
> >> (Although I am not quite sure
> >> when one is more desirable than the other)
> >
> > Most kafka's classes implementing Closeable/AutoCloseable doesn't throw checked exception in close() method. Perhaps we should have a "KafkaCloseable" interface which has a close() method without throwing any checked exception...
> >
> > On 2018/09/27 19:11:20, Yishun Guan <gy...@gmail.com> wrote:
> >> Hi All,
> >>
> >> Chia-Ping, I agree, similar to VarifiableConsumer, VarifiableProducer
> >> should be implementing Closeable as well (Although I am not quite sure
> >> when one is more desirable than the other), also I just looked through
> >> your list - these are some great additions, I will add them to the
> >> list.
> >>
> >> Thanks,
> >> Yishun
> >> On Thu, Sep 27, 2018 at 3:26 AM Dongjin Lee <do...@apache.org> wrote:
> >>>
> >>> Hi Yishun,
> >>>
> >>> Thank you for your great KIP. In fact, I have also encountered the cases
> >>> where Autoclosable is so desired several times! Let me inspect more
> >>> candidate classes as well.
> >>>
> >>> +1. I also refined your KIP a little bit.
> >>>
> >>> Best,
> >>> Dongjin
> >>>
> >>> On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <ch...@apache.org> wrote:
> >>>
> >>>> hi Yishun
> >>>>
> >>>> Thanks for nice KIP!
> >>>>
> >>>> Q1)
> >>>> Why VerifiableProducer extend Closeable rather than AutoCloseable?
> >>>>
> >>>> Q2)
> >>>> I grep project and then noticed there are other close methods but do not
> >>>> implement AutoCloseable.
> >>>> For example:
> >>>> 1) WorkerConnector
> >>>> 2) MemoryRecordsBuilder
> >>>> 3) MetricsReporter
> >>>> 4) ExpiringCredentialRefreshingLogin
> >>>> 5) KafkaChannel
> >>>> 6) ConsumerInterceptor
> >>>> 7) SelectorMetrics
> >>>> 8) HeartbeatThread
> >>>>
> >>>> Cheers,
> >>>> Chia-Ping
> >>>>
> >>>>
> >>>> On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
> >>>>> Hi All,
> >>>>>
> >>>>> Here is a trivial KIP:
> >>>>>
> >>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
> >>>>>
> >>>>> Suggestions are welcome.
> >>>>>
> >>>>> Thanks,
> >>>>> Yishun
> >>>>>
> >>>>
> >>>
> >>>
> >>> --
> >>> *Dongjin Lee*
> >>>
> >>> *A hitchhiker in the mathematical world.*
> >>>
> >>> *github: <http://goog_969573159/>github.com/dongjinleekr
> >>> <http://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
> >>> <http://kr.linkedin.com/in/dongjinleekr>slideshare:
> >>> www.slideshare.net/dongjinleekr
> >>> <http://www.slideshare.net/dongjinleekr>*
> >>
>
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate
classes that has close()
Posted by John Roesler <jo...@confluent.io>.
> Overall, it seems that `AutoClosable` might be the better interface to
> use though because it's more generic.
This sounds good to me. I don't know whether or not it's worth actually
transitioning any existing classes from Closeable to AutoCloseable.
I don't think it would affect any invocations, but there's the off-chance
that someone has assigned an instance to a Closeable variable, which would
break.
Overalll, it seems like maybe we should leave any Closeable implementations
alone and just add AutoCloseable where there's no existing closeable
interface.
-John
On Wed, Oct 3, 2018 at 12:06 PM Matthias J. Sax <ma...@confluent.io>
wrote:
> Thanks for clarifying. I thought that if we inherit `close() throws
> Exception` we need to declare the same exception -- this would have been
> an issue. Thus, my backward compatibility concerns are resolved.
>
> About try-with-resources: I think, allowing to use try-with-resources is
> the core motivation of this KIP to begin with. Also note, that `Closable
> extends AutoClosable`. Thus, both interfaces work with try-with-resource.
>
> Overall, it seems that `AutoClosable` might be the better interface to
> use though because it's more generic.
>
> -Matthias
>
>
> On 10/3/18 9:48 AM, Colin McCabe wrote:
> > On Sun, Sep 30, 2018, at 13:19, Matthias J. Sax wrote:
> >> Closeable is part of `java.io` while AutoClosable is part of
> >> `java.lang`. Thus, the second one is more generic. Also, JavaDoc points
> >> out that `Closable#close()` must be idempotent while
> >> `AutoClosable#close()` can have side effects.
> >
> > That's an interesting note. Looks like the exact JavaDoc text is:
> >
> > > Note that unlike the close method of Closeable, this close method is
> not
> > > required to be idempotent. In other words, calling this close method
> more
> > > than once may have some visible side effect, unlike Closeable.close
> which
> > > is required to have no effect if called more than once. However,
> > > implementers of this interface are strongly encouraged to make their
> close
> > > methods idempotent.
> >
> > So you can make it non-idempotent, but it's still recommended to make it
> idempotent.
> >
> >>
> >> Thus, I am not sure atm which one suits better.
> >>
> >> However, it's a good hint, that `AutoClosable#close()` declares `throws
> >> Exception` and thus, it seems to be a backward incompatible change.
> >> Hence, I am not sure if we can actually move forward easily with this
> KIP.
> >
> > I was worried about that too, but actually you can implement the
> AutoCloseable interface without declaring "throws Exception". In general,
> you can implement an interface while throwing a subset of the possible
> checked exceptions.
> >
> > There is one big benefit of AutoCloseable that I haven't seen mentioned
> here yet: the ability to use constructrs like try-with-resources
> transparently. So you can do things like
> >
> >> try (MyClass m = new MyClass()) {
> >> m.doSomething(...);
> >> }
> >
> > best,
> > Colin
> >
> >>
> >> Nit: `RecordCollectorImpl` is an internal class that implements
> >> `RecordCollector` -- should `RecordCollector extends AutoCloseable`?
> >>
> >>
> >> -Matthias
> >>
> >>
> >> On 9/27/18 7:46 PM, Chia-Ping Tsai wrote:
> >>>> (Although I am not quite sure
> >>>> when one is more desirable than the other)
> >>>
> >>> Most kafka's classes implementing Closeable/AutoCloseable doesn't
> throw checked exception in close() method. Perhaps we should have a
> "KafkaCloseable" interface which has a close() method without throwing any
> checked exception...
> >>>
> >>> On 2018/09/27 19:11:20, Yishun Guan <gy...@gmail.com> wrote:
> >>>> Hi All,
> >>>>
> >>>> Chia-Ping, I agree, similar to VarifiableConsumer, VarifiableProducer
> >>>> should be implementing Closeable as well (Although I am not quite sure
> >>>> when one is more desirable than the other), also I just looked through
> >>>> your list - these are some great additions, I will add them to the
> >>>> list.
> >>>>
> >>>> Thanks,
> >>>> Yishun
> >>>> On Thu, Sep 27, 2018 at 3:26 AM Dongjin Lee <do...@apache.org>
> wrote:
> >>>>>
> >>>>> Hi Yishun,
> >>>>>
> >>>>> Thank you for your great KIP. In fact, I have also encountered the
> cases
> >>>>> where Autoclosable is so desired several times! Let me inspect more
> >>>>> candidate classes as well.
> >>>>>
> >>>>> +1. I also refined your KIP a little bit.
> >>>>>
> >>>>> Best,
> >>>>> Dongjin
> >>>>>
> >>>>> On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <ch...@apache.org>
> wrote:
> >>>>>
> >>>>>> hi Yishun
> >>>>>>
> >>>>>> Thanks for nice KIP!
> >>>>>>
> >>>>>> Q1)
> >>>>>> Why VerifiableProducer extend Closeable rather than AutoCloseable?
> >>>>>>
> >>>>>> Q2)
> >>>>>> I grep project and then noticed there are other close methods but
> do not
> >>>>>> implement AutoCloseable.
> >>>>>> For example:
> >>>>>> 1) WorkerConnector
> >>>>>> 2) MemoryRecordsBuilder
> >>>>>> 3) MetricsReporter
> >>>>>> 4) ExpiringCredentialRefreshingLogin
> >>>>>> 5) KafkaChannel
> >>>>>> 6) ConsumerInterceptor
> >>>>>> 7) SelectorMetrics
> >>>>>> 8) HeartbeatThread
> >>>>>>
> >>>>>> Cheers,
> >>>>>> Chia-Ping
> >>>>>>
> >>>>>>
> >>>>>> On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
> >>>>>>> Hi All,
> >>>>>>>
> >>>>>>> Here is a trivial KIP:
> >>>>>>>
> >>>>>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
> >>>>>>>
> >>>>>>> Suggestions are welcome.
> >>>>>>>
> >>>>>>> Thanks,
> >>>>>>> Yishun
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> *Dongjin Lee*
> >>>>>
> >>>>> *A hitchhiker in the mathematical world.*
> >>>>>
> >>>>> *github: <http://goog_969573159/>github.com/dongjinleekr
> >>>>> <http://github.com/dongjinleekr>linkedin:
> kr.linkedin.com/in/dongjinleekr
> >>>>> <http://kr.linkedin.com/in/dongjinleekr>slideshare:
> >>>>> www.slideshare.net/dongjinleekr
> >>>>> <http://www.slideshare.net/dongjinleekr>*
> >>>>
> >>
> >> Email had 1 attachment:
> >> + signature.asc
> >> 1k (application/pgp-signature)
>
>
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate
classes that has close()
Posted by "Matthias J. Sax" <ma...@confluent.io>.
Thanks for clarifying. I thought that if we inherit `close() throws
Exception` we need to declare the same exception -- this would have been
an issue. Thus, my backward compatibility concerns are resolved.
About try-with-resources: I think, allowing to use try-with-resources is
the core motivation of this KIP to begin with. Also note, that `Closable
extends AutoClosable`. Thus, both interfaces work with try-with-resource.
Overall, it seems that `AutoClosable` might be the better interface to
use though because it's more generic.
-Matthias
On 10/3/18 9:48 AM, Colin McCabe wrote:
> On Sun, Sep 30, 2018, at 13:19, Matthias J. Sax wrote:
>> Closeable is part of `java.io` while AutoClosable is part of
>> `java.lang`. Thus, the second one is more generic. Also, JavaDoc points
>> out that `Closable#close()` must be idempotent while
>> `AutoClosable#close()` can have side effects.
>
> That's an interesting note. Looks like the exact JavaDoc text is:
>
> > Note that unlike the close method of Closeable, this close method is not
> > required to be idempotent. In other words, calling this close method more
> > than once may have some visible side effect, unlike Closeable.close which
> > is required to have no effect if called more than once. However,
> > implementers of this interface are strongly encouraged to make their close
> > methods idempotent.
>
> So you can make it non-idempotent, but it's still recommended to make it idempotent.
>
>>
>> Thus, I am not sure atm which one suits better.
>>
>> However, it's a good hint, that `AutoClosable#close()` declares `throws
>> Exception` and thus, it seems to be a backward incompatible change.
>> Hence, I am not sure if we can actually move forward easily with this KIP.
>
> I was worried about that too, but actually you can implement the AutoCloseable interface without declaring "throws Exception". In general, you can implement an interface while throwing a subset of the possible checked exceptions.
>
> There is one big benefit of AutoCloseable that I haven't seen mentioned here yet: the ability to use constructrs like try-with-resources transparently. So you can do things like
>
>> try (MyClass m = new MyClass()) {
>> m.doSomething(...);
>> }
>
> best,
> Colin
>
>>
>> Nit: `RecordCollectorImpl` is an internal class that implements
>> `RecordCollector` -- should `RecordCollector extends AutoCloseable`?
>>
>>
>> -Matthias
>>
>>
>> On 9/27/18 7:46 PM, Chia-Ping Tsai wrote:
>>>> (Although I am not quite sure
>>>> when one is more desirable than the other)
>>>
>>> Most kafka's classes implementing Closeable/AutoCloseable doesn't throw checked exception in close() method. Perhaps we should have a "KafkaCloseable" interface which has a close() method without throwing any checked exception...
>>>
>>> On 2018/09/27 19:11:20, Yishun Guan <gy...@gmail.com> wrote:
>>>> Hi All,
>>>>
>>>> Chia-Ping, I agree, similar to VarifiableConsumer, VarifiableProducer
>>>> should be implementing Closeable as well (Although I am not quite sure
>>>> when one is more desirable than the other), also I just looked through
>>>> your list - these are some great additions, I will add them to the
>>>> list.
>>>>
>>>> Thanks,
>>>> Yishun
>>>> On Thu, Sep 27, 2018 at 3:26 AM Dongjin Lee <do...@apache.org> wrote:
>>>>>
>>>>> Hi Yishun,
>>>>>
>>>>> Thank you for your great KIP. In fact, I have also encountered the cases
>>>>> where Autoclosable is so desired several times! Let me inspect more
>>>>> candidate classes as well.
>>>>>
>>>>> +1. I also refined your KIP a little bit.
>>>>>
>>>>> Best,
>>>>> Dongjin
>>>>>
>>>>> On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <ch...@apache.org> wrote:
>>>>>
>>>>>> hi Yishun
>>>>>>
>>>>>> Thanks for nice KIP!
>>>>>>
>>>>>> Q1)
>>>>>> Why VerifiableProducer extend Closeable rather than AutoCloseable?
>>>>>>
>>>>>> Q2)
>>>>>> I grep project and then noticed there are other close methods but do not
>>>>>> implement AutoCloseable.
>>>>>> For example:
>>>>>> 1) WorkerConnector
>>>>>> 2) MemoryRecordsBuilder
>>>>>> 3) MetricsReporter
>>>>>> 4) ExpiringCredentialRefreshingLogin
>>>>>> 5) KafkaChannel
>>>>>> 6) ConsumerInterceptor
>>>>>> 7) SelectorMetrics
>>>>>> 8) HeartbeatThread
>>>>>>
>>>>>> Cheers,
>>>>>> Chia-Ping
>>>>>>
>>>>>>
>>>>>> On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
>>>>>>> Hi All,
>>>>>>>
>>>>>>> Here is a trivial KIP:
>>>>>>>
>>>>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
>>>>>>>
>>>>>>> Suggestions are welcome.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Yishun
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> *Dongjin Lee*
>>>>>
>>>>> *A hitchhiker in the mathematical world.*
>>>>>
>>>>> *github: <http://goog_969573159/>github.com/dongjinleekr
>>>>> <http://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
>>>>> <http://kr.linkedin.com/in/dongjinleekr>slideshare:
>>>>> www.slideshare.net/dongjinleekr
>>>>> <http://www.slideshare.net/dongjinleekr>*
>>>>
>>
>> Email had 1 attachment:
>> + signature.asc
>> 1k (application/pgp-signature)
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate
classes that has close()
Posted by Yishun Guan <gy...@gmail.com>.
Hi Colin,
Yes, I absolutely agree with you. We can implement an interface while
throwing a subset of the possible checked exceptions, and empty set is also
a subset-so we also don't have to throw an exception.
And yes, you are right, I should emphasize the benefit of AutoCloseable in
the KIP.
Thanks!
Yishun
On Wed, Oct 3, 2018, 9:48 AM Colin McCabe <cm...@apache.org> wrote:
> On Sun, Sep 30, 2018, at 13:19, Matthias J. Sax wrote:
> > Closeable is part of `java.io` while AutoClosable is part of
> > `java.lang`. Thus, the second one is more generic. Also, JavaDoc points
> > out that `Closable#close()` must be idempotent while
> > `AutoClosable#close()` can have side effects.
>
> That's an interesting note. Looks like the exact JavaDoc text is:
>
> > Note that unlike the close method of Closeable, this close method is
> not
> > required to be idempotent. In other words, calling this close method
> more
> > than once may have some visible side effect, unlike Closeable.close
> which
> > is required to have no effect if called more than once. However,
> > implementers of this interface are strongly encouraged to make their
> close
> > methods idempotent.
>
> So you can make it non-idempotent, but it's still recommended to make it
> idempotent.
>
> >
> > Thus, I am not sure atm which one suits better.
> >
> > However, it's a good hint, that `AutoClosable#close()` declares `throws
> > Exception` and thus, it seems to be a backward incompatible change.
> > Hence, I am not sure if we can actually move forward easily with this
> KIP.
>
> I was worried about that too, but actually you can implement the
> AutoCloseable interface without declaring "throws Exception". In general,
> you can implement an interface while throwing a subset of the possible
> checked exceptions.
>
> There is one big benefit of AutoCloseable that I haven't seen mentioned
> here yet: the ability to use constructrs like try-with-resources
> transparently. So you can do things like
>
> > try (MyClass m = new MyClass()) {
> > m.doSomething(...);
> > }
>
> best,
> Colin
>
> >
> > Nit: `RecordCollectorImpl` is an internal class that implements
> > `RecordCollector` -- should `RecordCollector extends AutoCloseable`?
> >
> >
> > -Matthias
> >
> >
> > On 9/27/18 7:46 PM, Chia-Ping Tsai wrote:
> > >> (Although I am not quite sure
> > >> when one is more desirable than the other)
> > >
> > > Most kafka's classes implementing Closeable/AutoCloseable doesn't
> throw checked exception in close() method. Perhaps we should have a
> "KafkaCloseable" interface which has a close() method without throwing any
> checked exception...
> > >
> > > On 2018/09/27 19:11:20, Yishun Guan <gy...@gmail.com> wrote:
> > >> Hi All,
> > >>
> > >> Chia-Ping, I agree, similar to VarifiableConsumer, VarifiableProducer
> > >> should be implementing Closeable as well (Although I am not quite sure
> > >> when one is more desirable than the other), also I just looked through
> > >> your list - these are some great additions, I will add them to the
> > >> list.
> > >>
> > >> Thanks,
> > >> Yishun
> > >> On Thu, Sep 27, 2018 at 3:26 AM Dongjin Lee <do...@apache.org>
> wrote:
> > >>>
> > >>> Hi Yishun,
> > >>>
> > >>> Thank you for your great KIP. In fact, I have also encountered the
> cases
> > >>> where Autoclosable is so desired several times! Let me inspect more
> > >>> candidate classes as well.
> > >>>
> > >>> +1. I also refined your KIP a little bit.
> > >>>
> > >>> Best,
> > >>> Dongjin
> > >>>
> > >>> On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <ch...@apache.org>
> wrote:
> > >>>
> > >>>> hi Yishun
> > >>>>
> > >>>> Thanks for nice KIP!
> > >>>>
> > >>>> Q1)
> > >>>> Why VerifiableProducer extend Closeable rather than AutoCloseable?
> > >>>>
> > >>>> Q2)
> > >>>> I grep project and then noticed there are other close methods but
> do not
> > >>>> implement AutoCloseable.
> > >>>> For example:
> > >>>> 1) WorkerConnector
> > >>>> 2) MemoryRecordsBuilder
> > >>>> 3) MetricsReporter
> > >>>> 4) ExpiringCredentialRefreshingLogin
> > >>>> 5) KafkaChannel
> > >>>> 6) ConsumerInterceptor
> > >>>> 7) SelectorMetrics
> > >>>> 8) HeartbeatThread
> > >>>>
> > >>>> Cheers,
> > >>>> Chia-Ping
> > >>>>
> > >>>>
> > >>>> On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
> > >>>>> Hi All,
> > >>>>>
> > >>>>> Here is a trivial KIP:
> > >>>>>
> > >>>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
> > >>>>>
> > >>>>> Suggestions are welcome.
> > >>>>>
> > >>>>> Thanks,
> > >>>>> Yishun
> > >>>>>
> > >>>>
> > >>>
> > >>>
> > >>> --
> > >>> *Dongjin Lee*
> > >>>
> > >>> *A hitchhiker in the mathematical world.*
> > >>>
> > >>> *github: <http://goog_969573159/>github.com/dongjinleekr
> > >>> <http://github.com/dongjinleekr>linkedin:
> kr.linkedin.com/in/dongjinleekr
> > >>> <http://kr.linkedin.com/in/dongjinleekr>slideshare:
> > >>> www.slideshare.net/dongjinleekr
> > >>> <http://www.slideshare.net/dongjinleekr>*
> > >>
> >
> > Email had 1 attachment:
> > + signature.asc
> > 1k (application/pgp-signature)
>
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate classes
that has close()
Posted by Colin McCabe <cm...@apache.org>.
On Sun, Sep 30, 2018, at 13:19, Matthias J. Sax wrote:
> Closeable is part of `java.io` while AutoClosable is part of
> `java.lang`. Thus, the second one is more generic. Also, JavaDoc points
> out that `Closable#close()` must be idempotent while
> `AutoClosable#close()` can have side effects.
That's an interesting note. Looks like the exact JavaDoc text is:
> Note that unlike the close method of Closeable, this close method is not
> required to be idempotent. In other words, calling this close method more
> than once may have some visible side effect, unlike Closeable.close which
> is required to have no effect if called more than once. However,
> implementers of this interface are strongly encouraged to make their close
> methods idempotent.
So you can make it non-idempotent, but it's still recommended to make it idempotent.
>
> Thus, I am not sure atm which one suits better.
>
> However, it's a good hint, that `AutoClosable#close()` declares `throws
> Exception` and thus, it seems to be a backward incompatible change.
> Hence, I am not sure if we can actually move forward easily with this KIP.
I was worried about that too, but actually you can implement the AutoCloseable interface without declaring "throws Exception". In general, you can implement an interface while throwing a subset of the possible checked exceptions.
There is one big benefit of AutoCloseable that I haven't seen mentioned here yet: the ability to use constructrs like try-with-resources transparently. So you can do things like
> try (MyClass m = new MyClass()) {
> m.doSomething(...);
> }
best,
Colin
>
> Nit: `RecordCollectorImpl` is an internal class that implements
> `RecordCollector` -- should `RecordCollector extends AutoCloseable`?
>
>
> -Matthias
>
>
> On 9/27/18 7:46 PM, Chia-Ping Tsai wrote:
> >> (Although I am not quite sure
> >> when one is more desirable than the other)
> >
> > Most kafka's classes implementing Closeable/AutoCloseable doesn't throw checked exception in close() method. Perhaps we should have a "KafkaCloseable" interface which has a close() method without throwing any checked exception...
> >
> > On 2018/09/27 19:11:20, Yishun Guan <gy...@gmail.com> wrote:
> >> Hi All,
> >>
> >> Chia-Ping, I agree, similar to VarifiableConsumer, VarifiableProducer
> >> should be implementing Closeable as well (Although I am not quite sure
> >> when one is more desirable than the other), also I just looked through
> >> your list - these are some great additions, I will add them to the
> >> list.
> >>
> >> Thanks,
> >> Yishun
> >> On Thu, Sep 27, 2018 at 3:26 AM Dongjin Lee <do...@apache.org> wrote:
> >>>
> >>> Hi Yishun,
> >>>
> >>> Thank you for your great KIP. In fact, I have also encountered the cases
> >>> where Autoclosable is so desired several times! Let me inspect more
> >>> candidate classes as well.
> >>>
> >>> +1. I also refined your KIP a little bit.
> >>>
> >>> Best,
> >>> Dongjin
> >>>
> >>> On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <ch...@apache.org> wrote:
> >>>
> >>>> hi Yishun
> >>>>
> >>>> Thanks for nice KIP!
> >>>>
> >>>> Q1)
> >>>> Why VerifiableProducer extend Closeable rather than AutoCloseable?
> >>>>
> >>>> Q2)
> >>>> I grep project and then noticed there are other close methods but do not
> >>>> implement AutoCloseable.
> >>>> For example:
> >>>> 1) WorkerConnector
> >>>> 2) MemoryRecordsBuilder
> >>>> 3) MetricsReporter
> >>>> 4) ExpiringCredentialRefreshingLogin
> >>>> 5) KafkaChannel
> >>>> 6) ConsumerInterceptor
> >>>> 7) SelectorMetrics
> >>>> 8) HeartbeatThread
> >>>>
> >>>> Cheers,
> >>>> Chia-Ping
> >>>>
> >>>>
> >>>> On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
> >>>>> Hi All,
> >>>>>
> >>>>> Here is a trivial KIP:
> >>>>>
> >>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
> >>>>>
> >>>>> Suggestions are welcome.
> >>>>>
> >>>>> Thanks,
> >>>>> Yishun
> >>>>>
> >>>>
> >>>
> >>>
> >>> --
> >>> *Dongjin Lee*
> >>>
> >>> *A hitchhiker in the mathematical world.*
> >>>
> >>> *github: <http://goog_969573159/>github.com/dongjinleekr
> >>> <http://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
> >>> <http://kr.linkedin.com/in/dongjinleekr>slideshare:
> >>> www.slideshare.net/dongjinleekr
> >>> <http://www.slideshare.net/dongjinleekr>*
> >>
>
> Email had 1 attachment:
> + signature.asc
> 1k (application/pgp-signature)
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate
classes that has close()
Posted by Yishun Guan <gy...@gmail.com>.
Hi All,
I started a vote on this KIP on another email thread, but it didn't
generate responses. So I decided to start a vote on this thread. Thanks!
Link: https://www.zhihu.com/question/21491539
Best
Yishun
On Thu, Oct 4, 2018 at 11:37 PM Chia-Ping Tsai <ch...@apache.org> wrote:
> > However, it's a good hint, that `AutoClosable#close()` declares `throws
> > Exception` and thus, it seems to be a backward incompatible change.
> > Hence, I am not sure if we can actually move forward easily with this
> KIP.
>
> It is legal to remove declaration of "throwing Exception" from a class
> which extend AutoClosable, so method signature won't be changed. Hence it
> should be fine to backward compatibility.
>
> On 2018/09/30 20:19:57, "Matthias J. Sax" <ma...@confluent.io> wrote:
> > Closeable is part of `java.io` while AutoClosable is part of
> > `java.lang`. Thus, the second one is more generic. Also, JavaDoc points
> > out that `Closable#close()` must be idempotent while
> > `AutoClosable#close()` can have side effects.
> >
> > Thus, I am not sure atm which one suits better.
> >
> > However, it's a good hint, that `AutoClosable#close()` declares `throws
> > Exception` and thus, it seems to be a backward incompatible change.
> > Hence, I am not sure if we can actually move forward easily with this
> KIP.
> >
> > Nit: `RecordCollectorImpl` is an internal class that implements
> > `RecordCollector` -- should `RecordCollector extends AutoCloseable`?
> >
> >
> > -Matthias
> >
> >
> > On 9/27/18 7:46 PM, Chia-Ping Tsai wrote:
> > >> (Although I am not quite sure
> > >> when one is more desirable than the other)
> > >
> > > Most kafka's classes implementing Closeable/AutoCloseable doesn't
> throw checked exception in close() method. Perhaps we should have a
> "KafkaCloseable" interface which has a close() method without throwing any
> checked exception...
> > >
> > > On 2018/09/27 19:11:20, Yishun Guan <gy...@gmail.com> wrote:
> > >> Hi All,
> > >>
> > >> Chia-Ping, I agree, similar to VarifiableConsumer, VarifiableProducer
> > >> should be implementing Closeable as well (Although I am not quite sure
> > >> when one is more desirable than the other), also I just looked through
> > >> your list - these are some great additions, I will add them to the
> > >> list.
> > >>
> > >> Thanks,
> > >> Yishun
> > >> On Thu, Sep 27, 2018 at 3:26 AM Dongjin Lee <do...@apache.org>
> wrote:
> > >>>
> > >>> Hi Yishun,
> > >>>
> > >>> Thank you for your great KIP. In fact, I have also encountered the
> cases
> > >>> where Autoclosable is so desired several times! Let me inspect more
> > >>> candidate classes as well.
> > >>>
> > >>> +1. I also refined your KIP a little bit.
> > >>>
> > >>> Best,
> > >>> Dongjin
> > >>>
> > >>> On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <ch...@apache.org>
> wrote:
> > >>>
> > >>>> hi Yishun
> > >>>>
> > >>>> Thanks for nice KIP!
> > >>>>
> > >>>> Q1)
> > >>>> Why VerifiableProducer extend Closeable rather than AutoCloseable?
> > >>>>
> > >>>> Q2)
> > >>>> I grep project and then noticed there are other close methods but
> do not
> > >>>> implement AutoCloseable.
> > >>>> For example:
> > >>>> 1) WorkerConnector
> > >>>> 2) MemoryRecordsBuilder
> > >>>> 3) MetricsReporter
> > >>>> 4) ExpiringCredentialRefreshingLogin
> > >>>> 5) KafkaChannel
> > >>>> 6) ConsumerInterceptor
> > >>>> 7) SelectorMetrics
> > >>>> 8) HeartbeatThread
> > >>>>
> > >>>> Cheers,
> > >>>> Chia-Ping
> > >>>>
> > >>>>
> > >>>> On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
> > >>>>> Hi All,
> > >>>>>
> > >>>>> Here is a trivial KIP:
> > >>>>>
> > >>>>
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
> > >>>>>
> > >>>>> Suggestions are welcome.
> > >>>>>
> > >>>>> Thanks,
> > >>>>> Yishun
> > >>>>>
> > >>>>
> > >>>
> > >>>
> > >>> --
> > >>> *Dongjin Lee*
> > >>>
> > >>> *A hitchhiker in the mathematical world.*
> > >>>
> > >>> *github: <http://goog_969573159/>github.com/dongjinleekr
> > >>> <http://github.com/dongjinleekr>linkedin:
> kr.linkedin.com/in/dongjinleekr
> > >>> <http://kr.linkedin.com/in/dongjinleekr>slideshare:
> > >>> www.slideshare.net/dongjinleekr
> > >>> <http://www.slideshare.net/dongjinleekr>*
> > >>
> >
> >
>
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate classes that has close()
Posted by Chia-Ping Tsai <ch...@apache.org>.
> However, it's a good hint, that `AutoClosable#close()` declares `throws
> Exception` and thus, it seems to be a backward incompatible change.
> Hence, I am not sure if we can actually move forward easily with this KIP.
It is legal to remove declaration of "throwing Exception" from a class which extend AutoClosable, so method signature won't be changed. Hence it should be fine to backward compatibility.
On 2018/09/30 20:19:57, "Matthias J. Sax" <ma...@confluent.io> wrote:
> Closeable is part of `java.io` while AutoClosable is part of
> `java.lang`. Thus, the second one is more generic. Also, JavaDoc points
> out that `Closable#close()` must be idempotent while
> `AutoClosable#close()` can have side effects.
>
> Thus, I am not sure atm which one suits better.
>
> However, it's a good hint, that `AutoClosable#close()` declares `throws
> Exception` and thus, it seems to be a backward incompatible change.
> Hence, I am not sure if we can actually move forward easily with this KIP.
>
> Nit: `RecordCollectorImpl` is an internal class that implements
> `RecordCollector` -- should `RecordCollector extends AutoCloseable`?
>
>
> -Matthias
>
>
> On 9/27/18 7:46 PM, Chia-Ping Tsai wrote:
> >> (Although I am not quite sure
> >> when one is more desirable than the other)
> >
> > Most kafka's classes implementing Closeable/AutoCloseable doesn't throw checked exception in close() method. Perhaps we should have a "KafkaCloseable" interface which has a close() method without throwing any checked exception...
> >
> > On 2018/09/27 19:11:20, Yishun Guan <gy...@gmail.com> wrote:
> >> Hi All,
> >>
> >> Chia-Ping, I agree, similar to VarifiableConsumer, VarifiableProducer
> >> should be implementing Closeable as well (Although I am not quite sure
> >> when one is more desirable than the other), also I just looked through
> >> your list - these are some great additions, I will add them to the
> >> list.
> >>
> >> Thanks,
> >> Yishun
> >> On Thu, Sep 27, 2018 at 3:26 AM Dongjin Lee <do...@apache.org> wrote:
> >>>
> >>> Hi Yishun,
> >>>
> >>> Thank you for your great KIP. In fact, I have also encountered the cases
> >>> where Autoclosable is so desired several times! Let me inspect more
> >>> candidate classes as well.
> >>>
> >>> +1. I also refined your KIP a little bit.
> >>>
> >>> Best,
> >>> Dongjin
> >>>
> >>> On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <ch...@apache.org> wrote:
> >>>
> >>>> hi Yishun
> >>>>
> >>>> Thanks for nice KIP!
> >>>>
> >>>> Q1)
> >>>> Why VerifiableProducer extend Closeable rather than AutoCloseable?
> >>>>
> >>>> Q2)
> >>>> I grep project and then noticed there are other close methods but do not
> >>>> implement AutoCloseable.
> >>>> For example:
> >>>> 1) WorkerConnector
> >>>> 2) MemoryRecordsBuilder
> >>>> 3) MetricsReporter
> >>>> 4) ExpiringCredentialRefreshingLogin
> >>>> 5) KafkaChannel
> >>>> 6) ConsumerInterceptor
> >>>> 7) SelectorMetrics
> >>>> 8) HeartbeatThread
> >>>>
> >>>> Cheers,
> >>>> Chia-Ping
> >>>>
> >>>>
> >>>> On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
> >>>>> Hi All,
> >>>>>
> >>>>> Here is a trivial KIP:
> >>>>>
> >>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
> >>>>>
> >>>>> Suggestions are welcome.
> >>>>>
> >>>>> Thanks,
> >>>>> Yishun
> >>>>>
> >>>>
> >>>
> >>>
> >>> --
> >>> *Dongjin Lee*
> >>>
> >>> *A hitchhiker in the mathematical world.*
> >>>
> >>> *github: <http://goog_969573159/>github.com/dongjinleekr
> >>> <http://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
> >>> <http://kr.linkedin.com/in/dongjinleekr>slideshare:
> >>> www.slideshare.net/dongjinleekr
> >>> <http://www.slideshare.net/dongjinleekr>*
> >>
>
>
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate
classes that has close()
Posted by "Matthias J. Sax" <ma...@confluent.io>.
Closeable is part of `java.io` while AutoClosable is part of
`java.lang`. Thus, the second one is more generic. Also, JavaDoc points
out that `Closable#close()` must be idempotent while
`AutoClosable#close()` can have side effects.
Thus, I am not sure atm which one suits better.
However, it's a good hint, that `AutoClosable#close()` declares `throws
Exception` and thus, it seems to be a backward incompatible change.
Hence, I am not sure if we can actually move forward easily with this KIP.
Nit: `RecordCollectorImpl` is an internal class that implements
`RecordCollector` -- should `RecordCollector extends AutoCloseable`?
-Matthias
On 9/27/18 7:46 PM, Chia-Ping Tsai wrote:
>> (Although I am not quite sure
>> when one is more desirable than the other)
>
> Most kafka's classes implementing Closeable/AutoCloseable doesn't throw checked exception in close() method. Perhaps we should have a "KafkaCloseable" interface which has a close() method without throwing any checked exception...
>
> On 2018/09/27 19:11:20, Yishun Guan <gy...@gmail.com> wrote:
>> Hi All,
>>
>> Chia-Ping, I agree, similar to VarifiableConsumer, VarifiableProducer
>> should be implementing Closeable as well (Although I am not quite sure
>> when one is more desirable than the other), also I just looked through
>> your list - these are some great additions, I will add them to the
>> list.
>>
>> Thanks,
>> Yishun
>> On Thu, Sep 27, 2018 at 3:26 AM Dongjin Lee <do...@apache.org> wrote:
>>>
>>> Hi Yishun,
>>>
>>> Thank you for your great KIP. In fact, I have also encountered the cases
>>> where Autoclosable is so desired several times! Let me inspect more
>>> candidate classes as well.
>>>
>>> +1. I also refined your KIP a little bit.
>>>
>>> Best,
>>> Dongjin
>>>
>>> On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <ch...@apache.org> wrote:
>>>
>>>> hi Yishun
>>>>
>>>> Thanks for nice KIP!
>>>>
>>>> Q1)
>>>> Why VerifiableProducer extend Closeable rather than AutoCloseable?
>>>>
>>>> Q2)
>>>> I grep project and then noticed there are other close methods but do not
>>>> implement AutoCloseable.
>>>> For example:
>>>> 1) WorkerConnector
>>>> 2) MemoryRecordsBuilder
>>>> 3) MetricsReporter
>>>> 4) ExpiringCredentialRefreshingLogin
>>>> 5) KafkaChannel
>>>> 6) ConsumerInterceptor
>>>> 7) SelectorMetrics
>>>> 8) HeartbeatThread
>>>>
>>>> Cheers,
>>>> Chia-Ping
>>>>
>>>>
>>>> On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
>>>>> Hi All,
>>>>>
>>>>> Here is a trivial KIP:
>>>>>
>>>> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
>>>>>
>>>>> Suggestions are welcome.
>>>>>
>>>>> Thanks,
>>>>> Yishun
>>>>>
>>>>
>>>
>>>
>>> --
>>> *Dongjin Lee*
>>>
>>> *A hitchhiker in the mathematical world.*
>>>
>>> *github: <http://goog_969573159/>github.com/dongjinleekr
>>> <http://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
>>> <http://kr.linkedin.com/in/dongjinleekr>slideshare:
>>> www.slideshare.net/dongjinleekr
>>> <http://www.slideshare.net/dongjinleekr>*
>>
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate classes that has close()
Posted by Chia-Ping Tsai <ch...@apache.org>.
> (Although I am not quite sure
> when one is more desirable than the other)
Most kafka's classes implementing Closeable/AutoCloseable doesn't throw checked exception in close() method. Perhaps we should have a "KafkaCloseable" interface which has a close() method without throwing any checked exception...
On 2018/09/27 19:11:20, Yishun Guan <gy...@gmail.com> wrote:
> Hi All,
>
> Chia-Ping, I agree, similar to VarifiableConsumer, VarifiableProducer
> should be implementing Closeable as well (Although I am not quite sure
> when one is more desirable than the other), also I just looked through
> your list - these are some great additions, I will add them to the
> list.
>
> Thanks,
> Yishun
> On Thu, Sep 27, 2018 at 3:26 AM Dongjin Lee <do...@apache.org> wrote:
> >
> > Hi Yishun,
> >
> > Thank you for your great KIP. In fact, I have also encountered the cases
> > where Autoclosable is so desired several times! Let me inspect more
> > candidate classes as well.
> >
> > +1. I also refined your KIP a little bit.
> >
> > Best,
> > Dongjin
> >
> > On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <ch...@apache.org> wrote:
> >
> > > hi Yishun
> > >
> > > Thanks for nice KIP!
> > >
> > > Q1)
> > > Why VerifiableProducer extend Closeable rather than AutoCloseable?
> > >
> > > Q2)
> > > I grep project and then noticed there are other close methods but do not
> > > implement AutoCloseable.
> > > For example:
> > > 1) WorkerConnector
> > > 2) MemoryRecordsBuilder
> > > 3) MetricsReporter
> > > 4) ExpiringCredentialRefreshingLogin
> > > 5) KafkaChannel
> > > 6) ConsumerInterceptor
> > > 7) SelectorMetrics
> > > 8) HeartbeatThread
> > >
> > > Cheers,
> > > Chia-Ping
> > >
> > >
> > > On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
> > > > Hi All,
> > > >
> > > > Here is a trivial KIP:
> > > >
> > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
> > > >
> > > > Suggestions are welcome.
> > > >
> > > > Thanks,
> > > > Yishun
> > > >
> > >
> >
> >
> > --
> > *Dongjin Lee*
> >
> > *A hitchhiker in the mathematical world.*
> >
> > *github: <http://goog_969573159/>github.com/dongjinleekr
> > <http://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
> > <http://kr.linkedin.com/in/dongjinleekr>slideshare:
> > www.slideshare.net/dongjinleekr
> > <http://www.slideshare.net/dongjinleekr>*
>
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate
classes that has close()
Posted by Yishun Guan <gy...@gmail.com>.
Hi All,
Chia-Ping, I agree, similar to VarifiableConsumer, VarifiableProducer
should be implementing Closeable as well (Although I am not quite sure
when one is more desirable than the other), also I just looked through
your list - these are some great additions, I will add them to the
list.
Thanks,
Yishun
On Thu, Sep 27, 2018 at 3:26 AM Dongjin Lee <do...@apache.org> wrote:
>
> Hi Yishun,
>
> Thank you for your great KIP. In fact, I have also encountered the cases
> where Autoclosable is so desired several times! Let me inspect more
> candidate classes as well.
>
> +1. I also refined your KIP a little bit.
>
> Best,
> Dongjin
>
> On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <ch...@apache.org> wrote:
>
> > hi Yishun
> >
> > Thanks for nice KIP!
> >
> > Q1)
> > Why VerifiableProducer extend Closeable rather than AutoCloseable?
> >
> > Q2)
> > I grep project and then noticed there are other close methods but do not
> > implement AutoCloseable.
> > For example:
> > 1) WorkerConnector
> > 2) MemoryRecordsBuilder
> > 3) MetricsReporter
> > 4) ExpiringCredentialRefreshingLogin
> > 5) KafkaChannel
> > 6) ConsumerInterceptor
> > 7) SelectorMetrics
> > 8) HeartbeatThread
> >
> > Cheers,
> > Chia-Ping
> >
> >
> > On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
> > > Hi All,
> > >
> > > Here is a trivial KIP:
> > >
> > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
> > >
> > > Suggestions are welcome.
> > >
> > > Thanks,
> > > Yishun
> > >
> >
>
>
> --
> *Dongjin Lee*
>
> *A hitchhiker in the mathematical world.*
>
> *github: <http://goog_969573159/>github.com/dongjinleekr
> <http://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
> <http://kr.linkedin.com/in/dongjinleekr>slideshare:
> www.slideshare.net/dongjinleekr
> <http://www.slideshare.net/dongjinleekr>*
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate
classes that has close()
Posted by Dongjin Lee <do...@apache.org>.
Hi Yishun,
Thank you for your great KIP. In fact, I have also encountered the cases
where Autoclosable is so desired several times! Let me inspect more
candidate classes as well.
+1. I also refined your KIP a little bit.
Best,
Dongjin
On Thu, Sep 27, 2018 at 12:21 PM Chia-Ping Tsai <ch...@apache.org> wrote:
> hi Yishun
>
> Thanks for nice KIP!
>
> Q1)
> Why VerifiableProducer extend Closeable rather than AutoCloseable?
>
> Q2)
> I grep project and then noticed there are other close methods but do not
> implement AutoCloseable.
> For example:
> 1) WorkerConnector
> 2) MemoryRecordsBuilder
> 3) MetricsReporter
> 4) ExpiringCredentialRefreshingLogin
> 5) KafkaChannel
> 6) ConsumerInterceptor
> 7) SelectorMetrics
> 8) HeartbeatThread
>
> Cheers,
> Chia-Ping
>
>
> On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
> > Hi All,
> >
> > Here is a trivial KIP:
> >
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
> >
> > Suggestions are welcome.
> >
> > Thanks,
> > Yishun
> >
>
--
*Dongjin Lee*
*A hitchhiker in the mathematical world.*
*github: <http://goog_969573159/>github.com/dongjinleekr
<http://github.com/dongjinleekr>linkedin: kr.linkedin.com/in/dongjinleekr
<http://kr.linkedin.com/in/dongjinleekr>slideshare:
www.slideshare.net/dongjinleekr
<http://www.slideshare.net/dongjinleekr>*
Re: [DISCUSSION] KIP-376: Implement AutoClosable on appropriate classes that has close()
Posted by Chia-Ping Tsai <ch...@apache.org>.
hi Yishun
Thanks for nice KIP!
Q1)
Why VerifiableProducer extend Closeable rather than AutoCloseable?
Q2)
I grep project and then noticed there are other close methods but do not implement AutoCloseable.
For example:
1) WorkerConnector
2) MemoryRecordsBuilder
3) MetricsReporter
4) ExpiringCredentialRefreshingLogin
5) KafkaChannel
6) ConsumerInterceptor
7) SelectorMetrics
8) HeartbeatThread
Cheers,
Chia-Ping
On 2018/09/26 23:44:31, Yishun Guan <gy...@gmail.com> wrote:
> Hi All,
>
> Here is a trivial KIP:
> https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=93325308
>
> Suggestions are welcome.
>
> Thanks,
> Yishun
>