Hi Guozhang, Thanks for your review, let's exclude the "localThreadsMetadata returning StreamsMetadata" change then. This way, as well, this KIP is completely consistent on making those Metadata classes interfaces with internal implementations. I'll update the KIP document right away.
Thanks for replying on Saturday :) On Sat, Jun 5, 2021 at 4:22 PM Guozhang Wang <wangg...@gmail.com> wrote: > Hi Josep, > > I think the most significant change would be, "localThreadsMetadata" > returns a StreamsMetadata instead of ThreadMetadata, and StreamsMetadata > would expose a new API to return a set of ThreadMetadata. > > All others (including the repackaging and splitting of interface / impl) > are minor indeed. > > After a second thought, I feel it is not fair to squeeze in this > significant change into your KIP, without the community having a separating > discussion about it, so I think we can table it for now and only align on > the other minor things: 1) have a o.a.k.streams.StreamsMetadata interface > (along with an internal implementation class), 2) deprecate the > o.a.k.streams.state.StreamsMetadata class and also the corresponding caller > of Streams that returns this class. > > Guozhang > > On Fri, Jun 4, 2021 at 4:13 PM Josep Prat <josep.p...@aiven.io.invalid> > wrote: > > > Hi Guozhang, > > So if I understand correctly, it's only a couple of small changes that > need > > to be made to this KIP to be aligned with KAFKA-12370, right? > > > > I'm guessing that StreamsMetadata would not only moved to o.a.k.streams > but > > also be split with Interface + internal implementation, am I right? > > > > > > If that's the case, I could, most probably, update the KIP by Saturday > > afternoon CEST. > > > > Let me know if I understood you correctly. > > > > Thanks for the comments! > > > > ——— > > Josep Prat > > > > Aiven Deutschland GmbH > > > > Immanuelkirchstraße 26, 10405 Berlin > > > > Amtsgericht Charlottenburg, HRB 209739 B > > > > Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen > > > > m: +491715557497 > > > > w: aiven.io > > > > e: josep.p...@aiven.io > > > > On Sat, Jun 5, 2021, 00:11 Guozhang Wang <wangg...@gmail.com> wrote: > > > > > Hello Josep, > > > > > > Thanks for the proposal! The write-up looks good to me in general. I'm > > just > > > wondering if you feel comfortable to align this with another JIRA/KIP > > > further down the road: > > > > > > https://issues.apache.org/jira/browse/KAFKA-12370 > > > > > > Which tries to clean up the metadata hierarchy and the callers as > > > StreamsMetadata -> ThreadMetadata -> TaskMetadata, and most Streams > APIs > > > return the top-level StreamsMetadata. > > > > > > It just have slight differences with the current proposal: 1) instead > of > > > returning a ThreadMetadata, "localThreadsMetadata" returns > > > a StreamsMetadata, and 2) the `StreamsMetadata` would also be moved > from > > > o.a.k.streams.state to o.a.k.streams. > > > > > > What do you think about this? It's totally okay if you are not > > comfortable > > > changing or expanding the scope of this KIP, that's totally fine with > me > > as > > > well, and we can just change again in the future if necessary --- just > > > trying to see if we can align the direction on the first shot here :) > > > > > > > > > Guozhang > > > > > > On Fri, Jun 4, 2021 at 1:51 AM Bruno Cadonna <cado...@apache.org> > wrote: > > > > > > > Thanks, Josep! > > > > > > > > +1 (binding) > > > > > > > > Bruno > > > > > > > > On 04.06.21 10:27, Josep Prat wrote: > > > > > Hi all, > > > > > I'd like to call for a vote on KIP-744: Migrate TaskMetadata and > > > > > ThreadMetadata to an interface with internal implementation > > > > > KIP page can be found here: > > > https://cwiki.apache.org/confluence/x/XIrOCg > > > > > Discussion thread can be found here: > > > > > > > > > > > > > > > https://lists.apache.org/x/thread.html/r1d20fb6dbd6b01bb84cbb17e992f4d08308980dfc5f2e0a68d674413@%3Cdev.kafka.apache.org%3E > > > > > > > > > > As it was pointed out, hopefully this KIP can be approved before > the > > > 3.0 > > > > > deadline, as we can clean up some non naming compliant methods > > recently > > > > > introduced. > > > > > > > > > > > > > > > Please note that the scope of the KIP increased during the > discussion > > > to > > > > > also include ThreadMetadata. > > > > > > > > > > Thank you, > > > > > > > > > > > > > > > > > > -- > > > -- Guozhang > > > > > > > > -- > -- Guozhang > -- Josep Prat *Aiven Deutschland GmbH* Immanuelkirchstraße 26, 10405 Berlin Amtsgericht Charlottenburg, HRB 209739 B Geschäftsführer: Oskari Saarenmaa & Hannu Valtonen *m:* +491715557497 *w:* aiven.io *e:* josep.p...@aiven.io
