Xavier, Colin and Tom can you line up on this? I don't really mind which solution is chosen, but I think it needs to be done be before I can close the vote.
I want to help you with the implementation after a decision has been made. Just let me know. Thanks, Steven Op di 12 dec. 2017 om 03:54 schreef Colin McCabe <cmcc...@apache.org>: > Thanks, Xavier.... we should definitely think about what happens when > exceptions are thrown from these functions. > > I would suggest maybe we should just implement whenComplete, rather than > exposing addWaiter. addWaiter was never intended as a public API, and > it's a little weird. whenComplete is nice because it supports chaining, > and should be more familiar to users of other async APIs. > > best, > Colin > > > On Fri, Dec 8, 2017, at 16:26, Xavier Léauté wrote: > > Hi Steven, > > > > I noticed you are making KafkaFuture.addWaiter(...) public as part of > > your > > PR. This is a very useful method to add – and you should mention it in > > the > > KIP – however addWaiter currently doesn't guard against exceptions thrown > > inside of the BiConsumer function, which is something we should probably > > fix before making it public. > > > > I was about to make the necessary exception handling changes as part of > > https://github.com/apache/kafka/pull/4308 until someone pointed out your > > KIP to me. Since you already have a PR out, it might be worth > > incorporating > > my fixes (and the extra docs), what do you think? > > > > I'll rebase my PR onto yours to make it easier to merge. > > > > Thanks! > > Xavier > > > > > > On Mon, Dec 4, 2017 at 4:03 AM Steven Aerts <steven.ae...@gmail.com> > > wrote: > > > > > Tom, > > > > > > Thanks for the review. > > > updated the motivation a little bit, it's better, but I have to admit > can > > > be improved. > > > I made addWaiters public. > > > > > > Enjoy, > > > > > > Steven > > > > > > > > > > > > Op ma 4 dec. 2017 om 11:01 schreef Tom Bentley <t.j.bent...@gmail.com > >: > > > > > > > Hi Steven, > > > > > > > > Thanks for updating the KIP. I have a couple of points: > > > > > > > > 1. Typo in the first sentence of the Motivation. Also what does > "empty > > > > public abstract classes with one abstract method" mean -- if it's > got one > > > > abstract method in what way is it empty? > > > > 2.From an entirely self-centred point of view, the main thing that's > > > > missing for my work in KIP-183 is that addWaiter() needs to be > public. > > > > > > > > Thanks again, > > > > > > > > Tom > > > > > > > > On 2 December 2017 at 10:07, Steven Aerts <steven.ae...@gmail.com> > > > wrote: > > > > > > > > > Hi Tom, > > > > > > > > > > I just made changes to the proposal of KIP-218, to make everything > more > > > > > backwards compatible as suggested by Collin. > > > > > For me it is now in a state where starts to become final. > > > > > > > > > > I propose to wait a few days so everybody can take a look and open > the > > > > > votes when I do not receive any major comments. > > > > > > > > > > Does that sound ok for you? > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > 218%3A+Make+KafkaFuture.Function+java+8+lambda+compatible > > > > > > > > > > Thanks for your patience, > > > > > > > > > > > > > > > Steven > > > > > > > > > > > > > > > Op vr 1 dec. 2017 om 11:55 schreef Tom Bentley < > t.j.bent...@gmail.com > > > >: > > > > > > > > > > > Hi Steven, > > > > > > > > > > > > I'm particularly interested in seeing progress on this KIP as the > > > work > > > > > for > > > > > > KIP-183 needs a public version of BiConsumer. do you have any > idea > > > when > > > > > the > > > > > > KIP might be ready for voting? > > > > > > > > > > > > Thanks, > > > > > > > > > > > > Tom > > > > > > > > > > > > On 10 November 2017 at 13:38, Steven Aerts < > steven.ae...@gmail.com> > > > > > wrote: > > > > > > > > > > > > > Collin, Ben, > > > > > > > > > > > > > > Thanks for the input. > > > > > > > > > > > > > > I will work out this proposa, so I get an idea on the impact. > > > > > > > > > > > > > > Do you think it is a good idea to line up the new method names > with > > > > > those > > > > > > > of CompletableFuture? > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > > > > Steven > > > > > > > > > > > > > > Op vr 10 nov. 2017 om 12:12 schreef Ben Stopford < > b...@confluent.io > > > >: > > > > > > > > > > > > > > > Sounds like a good middle ground to me. What do you think > Steven? > > > > > > > > > > > > > > > > On Mon, Nov 6, 2017 at 8:18 PM Colin McCabe < > cmcc...@apache.org> > > > > > > wrote: > > > > > > > > > > > > > > > > > It would definitely be nice to use the jdk8 > > > CompletableFuture. I > > > > > > think > > > > > > > > > that's a bit of a separate discussion, though, since it has > > > such > > > > > > heavy > > > > > > > > > compatibility implications. > > > > > > > > > > > > > > > > > > How about making KIP-218 backwards compatible? As a > starting > > > > > point, > > > > > > > you > > > > > > > > > can change KafkaFuture#BiConsumer to an interface with no > > > > > > compatibility > > > > > > > > > implications, since there are currently no public functions > > > > exposed > > > > > > > that > > > > > > > > > use it. That leaves KafkaFuture#Function, which is > publicly > > > used > > > > > > now. > > > > > > > > > > > > > > > > > > For the purposes of KIP-218, how about adding a new > interface > > > > > > > > > FunctionInterface? Then you can add a function like this: > > > > > > > > > > > > > > > > > > > public abstract <R> KafkaFuture<R> > > > > > thenApply(FunctionInterface<T, > > > > > > R> > > > > > > > > > function); > > > > > > > > > > > > > > > > > > And mark the older declaration as deprecated: > > > > > > > > > > > > > > > > > > > @deprecated > > > > > > > > > > public abstract <R> KafkaFuture<R> > thenApply(Function<T, R> > > > > > > > function); > > > > > > > > > > > > > > > > > > This is a 100% compatible way to make things nicer for > java 8. > > > > > > > > > > > > > > > > > > cheers, > > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Nov 2, 2017, at 10:38, Steven Aerts wrote: > > > > > > > > > > Hi Tom, > > > > > > > > > > > > > > > > > > > > Nice observation. > > > > > > > > > > I changed "Rejected Alternatives" section to "Other > > > > > Alternatives", > > > > > > as > > > > > > > > > > I see myself as too much of an outsider to the kafka > > > community > > > > to > > > > > > be > > > > > > > > > > able to decide without this discussion. > > > > > > > > > > > > > > > > > > > > I see two major factors to decide: > > > > > > > > > > - how soon will KIP-118 (drop support of java 7) be > > > > implemented? > > > > > > > > > > - for which reasons do we drop backwards compatibility > for > > > > > public > > > > > > > > > > interfaces marked as Evolving > > > > > > > > > > > > > > > > > > > > If KIP-118 which is scheduled for version 2.0.0 is going > to > > > be > > > > > > > > > > implemented soon, I agree with you that replacing > KafkaFuture > > > > > with > > > > > > > > > > CompletableFuture (or CompletionStage) is a preferable > > > option. > > > > > > > > > > But as I am not familiar with the roadmap it is > difficult to > > > > tell > > > > > > for > > > > > > > > me. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Steven > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2017-11-02 11:27 GMT+01:00 Tom Bentley < > > > t.j.bent...@gmail.com > > > > >: > > > > > > > > > > > Hi Steven, > > > > > > > > > > > > > > > > > > > > > > I notice you've renamed the template's "Rejected > > > > Alternatives" > > > > > > > > section > > > > > > > > > to > > > > > > > > > > > "Other Alternatives", suggesting they're not rejected > yet > > > > (or, > > > > > if > > > > > > > you > > > > > > > > > have > > > > > > > > > > > rejected them, I think you should give your reasons). > > > > > > > > > > > > > > > > > > > > > > Personally, I'd like to understand the arguments > against > > > > simply > > > > > > > > > replacing > > > > > > > > > > > KafkaFuture with CompletableFuture in Kafka 2.0. In > other > > > > > words, > > > > > > if > > > > > > > > we > > > > > > > > > were > > > > > > > > > > > starting without needing to support Java 7 what would > be > > > the > > > > > > > > arguments > > > > > > > > > for > > > > > > > > > > > having our own KafkaFuture? > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > > > > > > > > > > > > > > Tom > > > > > > > > > > > > > > > > > > > > > > On 1 November 2017 at 16:01, Ted Yu < > yuzhih...@gmail.com> > > > > > wrote: > > > > > > > > > > > > > > > > > > > > > >> KAFKA-4423 is still open. > > > > > > > > > > >> When would Java 7 be dropped ? > > > > > > > > > > >> > > > > > > > > > > >> Thanks > > > > > > > > > > >> > > > > > > > > > > >> On Wed, Nov 1, 2017 at 8:56 AM, Ismael Juma < > > > > > ism...@juma.me.uk> > > > > > > > > > wrote: > > > > > > > > > > >> > > > > > > > > > > >> > On Wed, Nov 1, 2017 at 3:51 PM, Ted Yu < > > > > yuzhih...@gmail.com > > > > > > > > > > > > > > wrote: > > > > > > > > > > >> > > > > > > > > > > > >> > > bq. Wait for a kafka release which will not > support > > > > java 7 > > > > > > > > anymore > > > > > > > > > > >> > > > > > > > > > > > > >> > > Do you want to raise a separate thread for the > above ? > > > > > > > > > > >> > > > > > > > > > > > > >> > > > > > > > > > > > >> > There is already a KIP for this so a separate > thread is > > > > not > > > > > > > > needed. > > > > > > > > > > >> > > > > > > > > > > > >> > Ismael > > > > > > > > > > >> > > > > > > > > > > > >> > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
