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 <gyis...@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 <dong...@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 <chia7...@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 <gyis...@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)
signature.asc
Description: OpenPGP digital signature
