A summary of the changes from previous version can be seen under Public Interfaces --> UPDATE
On Sat, Jun 5, 2021 at 1:42 PM Josep Prat <josep.p...@aiven.io> wrote: > Hi all, > > During the vote thread there was a request to extend this KIP to be > aligned with https://issues.apache.org/jira/browse/KAFKA-12370. > Disucssion point is here: > https://lists.apache.org/x/thread.html/rafa347f2c0f0bddb253c1ddc8d2817d1f04d500bb96366a4f6ec9482@%3Cdev.kafka.apache.org%3E > > Pasting here my last message on that Thread: > > Hi Guozhang, > I updated the KIP with, but I'm not so sure about your point 1, as from the > user's perspective, the API available to them will be really different. If > you think it's fine to introduce this change for the users, I'm happy to do > that. If not, I can revert my KIP to remove your point 1). > > Link to the updated KIP: https://cwiki.apache.org/confluence/x/XIrOCg > > I don't know what's customary in these cases regarding the VOTE and DISCUSS > mail threads. Just in case, I will copy the e-mail exchange with a link to > this thread into the DISCUSS thread. > > > On Tue, Jun 1, 2021 at 6:37 PM Josep Prat <josep.p...@aiven.io> wrote: > >> Hi all, >> >> I managed to update the KIP to include the changes Bruno proposed and >> also extended it to include the migration of ThreadMetadata to an interface >> plus internal implementation. Please share any feedback or problems you >> might see with this approach. >> >> As the deadline for KIPs approaches, I plan to submit this KIP for voting >> on Friday, unless issues arise. >> >> Let me know if this makes sense, as this is my first KIP. >> >> Thanks in advance. >> >> On Tue, Jun 1, 2021 at 3:43 PM Josep Prat <josep.p...@aiven.io> wrote: >> >>> Hi Bruno, >>> >>> Thanks for your feedback. I think it makes sense to add the hasCode() >>> and equals() definitions with its guarantees on the interface level. >>> >>> I'm also inclined to add the ThreadMetadata conversion to an interface >>> into this KIP. Implementation-wise is not that different. And on API level >>> modification is at a similar complexity as the current proposed change. >>> I'll shortly update the KIP. >>> >>> Best, >>> >>> >>> >>> On Tue, Jun 1, 2021 at 3:09 PM Bruno Cadonna <cado...@apache.org> wrote: >>> >>>> Hi Josep, >>>> >>>> Thank you for the KIP! >>>> >>>> Specifying TaskMetadata as an interface lifts the guarantee that >>>> equals() and hashCode() on the TaskMetadata return consistent values >>>> for >>>> the same content. Note that the current TaskMetadata implements both >>>> methods, so theoretically users might already rely on them. >>>> >>>> The List interface in Java defines its equals()[1] and hashCode()[2] >>>> methods specifically and all correct implementations must adhere to >>>> that >>>> definition. >>>> >>>> Maybe it makes sense to add such a guarantee to the TaskMetadata >>>> interface on which users can rely similarly to the Java's List >>>> interface. >>>> >>>> Regarding your question about ThreadMetadata, I had the same thought. I >>>> would add it to the KIP unless the extended KIP risks not to be >>>> accepted >>>> before KIP freeze for 3.0. >>>> >>>> Best, >>>> Bruno >>>> >>>> >>>> [1] >>>> >>>> https://docs.oracle.com/javase/8/docs/api/java/util/List.html#equals-java.lang.Object- >>>> [2] >>>> https://docs.oracle.com/javase/8/docs/api/java/util/List.html#hashCode-- >>>> >>>> >>>> >>>> >>>> On 31.05.21 12:36, Josep Prat wrote: >>>> > 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 >>> >> >> >> -- >> >> 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
