Thanks Sophie, I like the current proposal better compared to adding a new TaskInfo class. +1 !
Guozhang On Wed, May 19, 2021 at 4:58 PM Sophie Blee-Goldman <sop...@confluent.io.invalid> wrote: > Just a friendly ping to please check out the finalized proposal of the KIP > and (re)cast your votes > > Thanks! > Sophie > > On Sun, May 16, 2021 at 7:28 PM Sophie Blee-Goldman <sop...@confluent.io> > wrote: > > > Thanks John. I have moved the discussion over to a [DISCUSS] thread, > where > > it should have been taking place all > > along. I'll officially kick off the vote again, but since this KIP has > > been through a significant overhauled since it's initial > > proposal, the previous votes cast will be invalidated. Please make a pass > > on the latest KIP and (re)cast your vote. > > > > If you have any concerns or comments beyond just small questions, please > > take them to the discussion thread. > > > > Thanks! > > Sophie > > > > On Fri, May 14, 2021 at 10:12 AM John Roesler <vvcep...@apache.org> > wrote: > > > >> Thanks for these updates, Sophie, > >> > >> Unfortunately, I have some minor suggestions: > >> > >> 1. "Topic Group" is a vestigial term from the early days of > >> the codebase. We call a "topic group" a "subtopology" in the > >> public interface now (although "topic group" is still used > >> internally some places). For user-facing consistency, we > >> should also use "subtopologyId" in your proposal. > >> > >> 2. I'm wondering if it's really necessary to introduce this > >> interface at all. I think your motivation is to be able to > >> get the subtopologyId and partition via TaskMetadata, right? > >> Why not just add those methods to TaskMetadata? Stepping > >> back, the concept of metadata about an identifier is a bit > >> elaborate. > >> > >> Sorry for thrashing what you were hoping would be a quick, > >> uncontroversial KIP. > >> > >> Thanks for your consideration, > >> John > >> > >> On Thu, 2021-05-13 at 19:35 -0700, Sophie Blee-Goldman > >> wrote: > >> > One last update: we will not actually remove the existing > >> > o.a.k.streams.processor.TaskId class, but only > >> > deprecate it, along with any methods that returned it (ie the getters > on > >> > ProcessorContext and StateStoreContext) > >> > > >> > Internally, everything will still be converted to use the new internal > >> > TaskId class, and public TaskIdMetadata interface, > >> > where appropriate. > >> > > >> > > >> > > >> > On Thu, May 13, 2021 at 6:42 PM Sophie Blee-Goldman < > >> sop...@confluent.io> > >> > wrote: > >> > > >> > > Thanks all. I updated the KIP slightly since there is some ambiguity > >> > > around whether the existing TaskId class is > >> > > currently part of the public API or not. To settle the matter, I > have > >> > > introduced a new public TaskId interface that > >> > > exposes the metadata, and moved the existing TaskId class to the > >> internals > >> > > package. The KIP <https://cwiki.apache.org/confluence/x/vYTOCg> has > >> been > >> > > updated > >> > > with the proposed API changes. > >> > > > >> > > @Guozhang Wang <guozh...@confluent.io> : I decided to leave this > new > >> > > TaskId interface in o.a.k.streams.processor since that's where the > >> > > TaskMetadata class is, along with the other related metadata classes > >> (eg > >> > > ThreadMetadata). I do agree it makes > >> > > more sense for them to be under o.a.k.streams, but I'd rather leave > >> them > >> > > together for now. > >> > > > >> > > Please let me know if there are any concerns, or you want to redact > >> your > >> > > vote :) > >> > > > >> > > -Sophie > >> > > > >> > > On Thu, May 13, 2021 at 3:11 PM Guozhang Wang <wangg...@gmail.com> > >> wrote: > >> > > > >> > > > +1 > >> > > > > >> > > > On a hindsight, maybe TaskId should not really be in > >> > > > `org.apache.kafka.streams.processor` but rather just in > >> `o.a.k.streams`, > >> > > > but maybe not worth pulling it up now :) > >> > > > > >> > > > Guozhang > >> > > > > >> > > > On Thu, May 13, 2021 at 1:58 PM Walker Carlson > >> > > > <wcarl...@confluent.io.invalid> wrote: > >> > > > > >> > > > > +1 from me! (non-binding) > >> > > > > > >> > > > > Walker > >> > > > > > >> > > > > On Thu, May 13, 2021 at 1:53 PM Sophie Blee-Goldman > >> > > > > <sop...@confluent.io.invalid> wrote: > >> > > > > > >> > > > > > Hey all, > >> > > > > > > >> > > > > > I'm just going to take this KIP straight to a vote since it > >> should be > >> > > > a > >> > > > > > trivial and uncontroversial change. Of course please raise any > >> > > > concerns > >> > > > > > should they come up, and I can take things to a DISCUSS > thread. > >> > > > > > > >> > > > > > The KIP is a simple change to move from String to TaskId for > the > >> > > > taskID > >> > > > > > field of TaskMetadata. > >> > > > > > > >> > > > > > KIP-740: Use TaskId instead of String for the taskId field in > >> > > > > TaskMetadata > >> > > > > > < > >> > > > > > > >> > > > > > >> > > > > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-740%3A+Use+TaskId+instead+of+String+for+the+taskId+field+in+TaskMetadata > >> > > > > > > > >> > > > > > > >> > > > > > Cheers, > >> > > > > > Sophie > >> > > > > > > >> > > > > > >> > > > > >> > > > > >> > > > -- > >> > > > -- Guozhang > >> > > > > >> > > > >> > >> > >> > -- -- Guozhang
