Thanks Walker. Overall, LGTM. However, I am wondering if we should have default implementations for both handler methods? Before the latest change, there was only one method and having a default was not necessary. However, forcing people to implement both methods might not be the best user experience: for example, if there is no global thread, one should not need to implement the global handler method (and the other way around).
Thus, it might be good to add default for both methods. If we add defaults, we should explain the default behavior to the KIP. -Matthias On 10/12/20 2:32 PM, Walker Carlson wrote: > Hello all, > > I just wanted to let you know that I had to make 2 minor updates to the KIP. > > 1) I changed the behavior of the shutdown client to not leave the client in > Error but instead close directly because this aligns better with our > state machine. > > 2) I added a separate call back for the global thread as it does not have > all the options as a streamThread does. i.e. replace. The default will be > to close the client. that will also be the only option as that is the > current behavior for the global thread. > > you can find the diff here: > https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=158876566&originalVersion=21&revisedVersion=23 > > If you have any problems with these changes let me know and we can discuss > them further > > Thank you, > Walker > > On Wed, Sep 30, 2020 at 7:33 AM Walker Carlson <wcarl...@confluent.io> > wrote: > >> >> Bruno Cadonna <br...@confluent.io> >> 4:51 AM (2 hours ago) >> to dev >> Thank you all for voting! >> >> This KIP is accepted with +3 binding (Guozhang, Bill, Matthias) and +2 >> non-binding (Bruno, Leah). >> >> Matthias, we will take care of the global threads, and for the >> replacement that will depend on Kip-663. >> >> Best, >> >> On Wed, Sep 30, 2020 at 4:59 AM Bruno Cadonna <br...@confluent.io> wrote: >> >>> Thanks a lot Walker! >>> >>> +1 (non-binding) >>> >>> Best, >>> Bruno >>> >>> On 30.09.20 03:10, Matthias J. Sax wrote: >>>> Thanks Walker. The proposed API changes LGTM. >>>> >>>> +1 (binding) >>>> >>>> One minor nit: you should also mention the global-thread that also needs >>>> to be shutdown if requested by the user. >>>> >>>> Minor side question: should we actually terminate a thread and create a >>>> new one, or instead revive the existing thread (reusing its existing >>> ID)? >>>> >>>> >>>> -Matthias >>>> >>>> On 9/29/20 2:39 PM, Bill Bejeck wrote: >>>>> Thanks for the KIP Walker. >>>>> >>>>> +1 (binding) >>>>> >>>>> -Bill >>>>> >>>>> On Tue, Sep 29, 2020 at 4:59 PM Guozhang Wang <wangg...@gmail.com> >>> wrote: >>>>> >>>>>> +1 again on the KIP. >>>>>> >>>>>> On Tue, Sep 29, 2020 at 1:51 PM Leah Thomas <ltho...@confluent.io> >>> wrote: >>>>>> >>>>>>> Hey Walker, >>>>>>> >>>>>>> Thanks for the KIP! I'm +1, non-binding. >>>>>>> >>>>>>> Cheers, >>>>>>> Leah >>>>>>> >>>>>>> On Tue, Sep 29, 2020 at 1:56 PM Walker Carlson < >>> wcarl...@confluent.io> >>>>>>> wrote: >>>>>>> >>>>>>>> Hello all, >>>>>>>> >>>>>>>> I made some changes to the KIP the descriptions are on the >>> discussion >>>>>>>> thread. If you have already voted I would ask you to confirm your >>> vote. >>>>>>>> >>>>>>>> Otherwise please vote so we can get this feature in. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Walker >>>>>>>> >>>>>>>> On Thu, Sep 24, 2020 at 4:36 PM John Roesler <vvcep...@apache.org> >>>>>>> wrote: >>>>>>>> >>>>>>>>> Thanks for the KIP, Walker! >>>>>>>>> >>>>>>>>> I’m +1 (binding) >>>>>>>>> >>>>>>>>> -John >>>>>>>>> >>>>>>>>> On Mon, Sep 21, 2020, at 17:04, Guozhang Wang wrote: >>>>>>>>>> Thanks for finalizing the KIP. +1 (binding) >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> Guozhang >>>>>>>>>> >>>>>>>>>> On Mon, Sep 21, 2020 at 1:38 PM Walker Carlson < >>>>>>> wcarl...@confluent.io> >>>>>>>>>> wrote: >>>>>>>>>> >>>>>>>>>>> Hello all, >>>>>>>>>>> >>>>>>>>>>> I would like to start a thread to vote for KIP-671 to add a >>>>>> method >>>>>>> to >>>>>>>>> close >>>>>>>>>>> all clients in a kafka streams application. >>>>>>>>>>> >>>>>>>>>>> KIP: >>>>>>>>>>> >>>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-671%3A+Shutdown+Streams+Application+when+appropriate+exception+is+thrown >>>>>>>>>>> >>>>>>>>>>> Discussion thread: *here >>>>>>>>>>> < >>>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>> https://mail-archives.apache.org/mod_mbox/kafka-dev/202009.mbox/%3CCAC55fuh3HAGCxz-PzxTJraczy6T-os2oiCV328PBeuJQSVYASg%40mail.gmail.com%3E >>>>>>>>>>>> * >>>>>>>>>>> >>>>>>>>>>> Thanks, >>>>>>>>>>> -Walker >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> -- >>>>>>>>>> -- Guozhang >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> -- >>>>>> -- Guozhang >>>>>> >>>>> >>> >> >
