Hi there, I updated the KIP page with Sophie's feedback. As she already mentioned, the intention would be to include this KIP in release 3.0.0 so we can avoid a deprecation cycle for the getTaskID method introduced in KIP-740, I hope I managed to capture this in the KIP description.
Just adding the link again for convenience: https://cwiki.apache.org/confluence/x/XIrOCg Thanks in advance, On Thu, May 27, 2021 at 10:08 PM Josep Prat <josep.p...@aiven.io> wrote: > Hi Sophie, > > Thanks for the feedback, I'll update the KIP tomorrow with your feedback. > They are all good points, and you are right, my phrasing could be > misleading. > > > Best, > > On Thu, May 27, 2021 at 10:02 PM Sophie Blee-Goldman > <sop...@confluent.io.invalid> wrote: > >> Thanks for the KIP! I'm on board with the overall proposal, just a few >> comments: >> >> 1) The motivation section says >> >> TaskMetadata should have never been a class available for the general >> > public, but more of an internal class >> >> >> which is a bit misleading as it seems to imply that TaskMetadata itself >> was >> never meant to be part of the public API >> at all. It might be better to phrase this as "TaskMetadata was never >> intended to be a public class that a user might >> need to instantiate, but rather an API for exposing metadata which is >> better served as an interface" --- or something >> to that effect. >> >> 2) You touch on this in a later section, but it would be good to call out >> directly in the *Public Interfaces* section that >> you are proposing to remove the `public TaskId getTaskId()` method that we >> added in KIP-740. Also I just want to >> note that to do so will require getting this KIP into 3.0, otherwise we'll >> need to go through a deprecation cycle for >> that API. I don't anticipate this being a problem as KIP freeze is still >> two weeks away, but it would be good to clarify. >> >> 3) nit: we should put the new internal implementation class under >> the org.apache.kafka.streams.processor.internals >> package instead of under org.apache.kafka.streams.internals. But this is >> an >> implementation detail and as such >> doesn't need to be covered by the KIP in the first place. >> >> - Sophie >> >> On Thu, May 27, 2021 at 1:55 AM Josep Prat <josep.p...@aiven.io.invalid> >> wrote: >> >> > I deliberately picked the most conservative approach of creating a new >> > Interface, instead of transforming the current class into an interface. >> > Feedback is most welcome! >> > >> > Best, >> > >> > On Thu, May 27, 2021 at 10:26 AM Josep Prat <josep.p...@aiven.io> >> wrote: >> > >> > > Hi there, >> > > I would like to propose KIP-744, to introduce TaskMetadata as an >> > > interface, to keep the its implementation as internal use. >> > > This KIP can be seen as a spin-off of KIP-740. >> > > >> > > https://cwiki.apache.org/confluence/x/XIrOCg >> > > >> > > Best, >> > > -- >> > > >> > > 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 >> > > >> > >> > >> > -- >> > >> > 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 >> > >> > > > -- > > 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 > -- 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
