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)