Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2018-01-21 Thread Steven Aerts
Xavier, Ismael, I think I updated everything. The KIP has been lined up with the latest version of the PR, it is free for everyone to be reviewed. Thanks for your feedback, Steven Op do 18 jan. 2018 om 13:10 schreef Steven Aerts : > Ok, will cook something up by the end of the weekend.

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2018-01-18 Thread Steven Aerts
Ok, will cook something up by the end of the weekend. Op wo 17 jan. 2018 om 18:36 schreef Xavier Léauté : > Hi Steven, do you think you'll get a chance to address the points Ismael > made? It'd be great to get this change into 1.1. > > Thanks! > Xavier > > On Tue, Dec 19, 2017 at 12:20 PM Ismael

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2018-01-17 Thread Xavier Léauté
Hi Steven, do you think you'll get a chance to address the points Ismael made? It'd be great to get this change into 1.1. Thanks! Xavier On Tue, Dec 19, 2017 at 12:20 PM Ismael Juma wrote: > Hi Steven, > > As a general rule, we don't freeze KIPs after the vote passes. It's > reasonably common f

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-19 Thread Ismael Juma
Hi Steven, As a general rule, we don't freeze KIPs after the vote passes. It's reasonably common for things to come up during code review, for example. If we think of improvements, we shouldn't refrain from doing them because of of the vote. If we do minor changes after the KIP passes, we usually

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-19 Thread Steven Aerts
Hello Ismael. Thanks for you feedback. > 1. The KIP seems to rely on the pull request for some of the details of the > proposal. Generally, the KIP should stand on its own. Looking back at what I wrote in the KIP, I agree that its style is too descriptive and relies too much on the content of t

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-19 Thread Ismael Juma
Thanks for the KIP, Steven. A few minor comments: 1. The KIP seems to rely on the pull request for some of the details of the proposal. Generally, the KIP should stand on its own. For example, the public interfaces section should include the signature of interfaces and methods being added. 2. Do

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-12 Thread Xavier Léauté
I'm fine with the whenComplete solution as well. On Tue, Dec 12, 2017 at 03:57 Tom Bentley wrote: > Hi Steven, > > I am happy with adding whenComplete() instead of addWaiter(), > > Cheers, > > Tom > > On 12 December 2017 at 11:11, Steven Aerts wrote: > > > Xavier, Colin and Tom > > > > can you l

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-12 Thread Tom Bentley
Hi Steven, I am happy with adding whenComplete() instead of addWaiter(), Cheers, Tom On 12 December 2017 at 11:11, Steven Aerts wrote: > 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 t

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-12 Thread Steven Aerts
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

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-11 Thread Colin McCabe
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 ni

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-08 Thread Xavier Léauté
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 shoul

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-04 Thread Steven Aerts
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 : > Hi Steven, > > Thanks for updating the KIP. I have a couple of points: > > 1. Typo in

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-04 Thread Tom Bentley
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 vie

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-02 Thread Steven Aerts
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 comme

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-12-01 Thread Tom Bentley
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 wrote: > Collin, Ben, > > Thanks for the inpu

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-11-10 Thread Steven Aerts
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 : > Sounds like a good middle g

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-11-10 Thread Ben Stopford
Sounds like a good middle ground to me. What do you think Steven? On Mon, Nov 6, 2017 at 8:18 PM Colin McCabe 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. >

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-11-06 Thread Colin McCabe
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

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-11-02 Thread Steven Aerts
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 impl

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-11-02 Thread Tom Bentley
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 KafkaFutu

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-11-01 Thread Ted Yu
KAFKA-4423 is still open. When would Java 7 be dropped ? Thanks On Wed, Nov 1, 2017 at 8:56 AM, Ismael Juma wrote: > On Wed, Nov 1, 2017 at 3:51 PM, Ted Yu wrote: > > > bq. Wait for a kafka release which will not support java 7 anymore > > > > Do you want to raise a separate thread for the abo

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-11-01 Thread Ismael Juma
On Wed, Nov 1, 2017 at 3:51 PM, Ted Yu 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

Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-11-01 Thread Ted Yu
bq. PR#4033 makes from those abstract classes interfaces. Please adjust the syntax. bq. Wait for a kafka release which will not support java 7 anymore Do you want to raise a separate thread for the above ? On Wed, Nov 1, 2017 at 7:50 AM, Steven Aerts

[DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible

2017-11-01 Thread Steven Aerts
Hey everybody, I made a KIP to improve working with the KafkaFuture class from java 8. https://cwiki.apache.org/confluence/display/KAFKA/KIP-218%3A+Make+KafkaFuture.Function+java+8+lambda+compatible All comments are more than welcome. Thanks, Steven