I'm starting to wonder if we should make ThreadMetadata follow the same path as TaskMetadata and be converted to an interface + internal implementation. What are your thoughts?
Thanks in advance, Josep On Mon, May 31, 2021 at 12:10 PM Josep Prat <josep.p...@aiven.io> wrote: > Hi there, > While looking closely at the classes affected by this change, I realized > that ThreadMetadata has a reference in its public API to the soon to be > deprecated TaskMetadata. For this reason, I updated the KIP to reflect that > 2 more changes are needed for this KIP: > ThreadMetadata: > - Deprecate activeTasks() and standbyTasks() > - Add getActiveTasks() and getStandbyTasks() that return the new Interface > instead. > > I also wrote it on the KIP, but this thing crossed my mind, do we need to > keep source compatibility for this particular change? > > > I would be really grateful if you could provide any feedback on the KIP ( > https://cwiki.apache.org/confluence/x/XIrOCg) > > Thanks in advance, > > On Fri, May 28, 2021 at 10:24 AM Josep Prat <josep.p...@aiven.io> wrote: > >> 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 >> > > > -- > > 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
