Hello Sophie, Thanks for your feedback. I have made all the suggested changes.
One note, on how users can accomplish this in today's world , I have made up this example and have never tried myself before. But I am assuming it will work. Let me know what you think. Thanks! Sagar. On Thu, Aug 18, 2022 at 7:17 AM Sophie Blee-Goldman <sop...@confluent.io.invalid> wrote: > Hey Sagar, thanks for the KIP! > > Just some cosmetic points to make it absolutely clear what this KIP is > doing: > 1) could you clarify up front in the Motivation section that this is > focused on Kafka Streams applications, and not the plain Producer client? > 2) you included the entire implementation of the `#send` method to > demonstrate the change in logic, but can you either remove the parts of > the implementation that aren't being touched here or at least highlight in > some way the specific lines that have changed? > 3) In general the implementation is, well, an implementation detail that > doesn't need to be included in the KIP, but it's ok -- always nice to get a > sense of how things will work internally. But what I think would be more > useful to show in the KIP is how things will work with the new public > interface -- ie, can you provide a brief example of how a user would go > about taking advantage of this new interface? Even better, include an > example of what it takes for a user to accomplish this behavior before this > KIP. It would help showcase the concrete benefit this KIP is bringing and > anchor the motivation section a bit better. > > Also nit: in the 2nd sentence under Public Interfaces I think it should say > "it would invoke the partition() method" -- ie this should be > "partition()" not "partition*s*()" > > Other than that this looks good, but I'll wait until you've addressed the > above to cast a vote. > > Thanks! > Sophie > > On Fri, Aug 12, 2022 at 10:36 PM Sagar <sagarmeansoc...@gmail.com> wrote: > > > Hey John, > > > > Thanks for the vote. I added the reason for the rejection of the > > alternatives. The first one is basically an option to broadcast to all > > partitions which I felt was restrictive. Instead the KIP allows > > multicasting to 0-N partitions based upon the partitioner implementation. > > > > Thanks! > > Sagar. > > > > On Sat, Aug 13, 2022 at 7:35 AM John Roesler <vvcep...@apache.org> > wrote: > > > > > Thanks, Sagar! > > > > > > I’m +1 (binding) > > > > > > Can you add a short explanation to each rejected alternative? I was > > > wondering why we wouldn’t provide an overloaded to()/addSink() (the > first > > > rejected alternative), and I had to look back at the Streams code to > see > > > that they both already accept the partitioner (I thought it was a > > config). > > > > > > Thanks! > > > -John > > > > > > On Tue, Aug 9, 2022, at 13:44, Walker Carlson wrote: > > > > +1 (non binding) > > > > > > > > Walker > > > > > > > > On Tue, May 31, 2022 at 4:44 AM Sagar <sagarmeansoc...@gmail.com> > > wrote: > > > > > > > >> Hi All, > > > >> > > > >> I would like to start a voting thread on > > > >> > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=211883356 > > > >> . > > > >> > > > >> I am just starting this as the discussion thread has been open for > 10+ > > > >> days. In case there are some comments, we can always discuss them > over > > > >> there. > > > >> > > > >> Thanks! > > > >> Sagar. > > > >> > > > > > >
