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 >> > > > >> > > >> >> >>
