Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@kafka.apache.org
MIME-Version: 1.0
References: <CAJZFVeCfnnyhq2Dfuq5B=VofDHjQJZ4jgbc70vE4gK0OE2HfNA@mail.gmail.com>
 <CALte62zhmUeKtkQn55DhLzyJQ9CP2-QcoAtyma87pE2fzyHX2w@mail.gmail.com>
 <CAD5tkZZVDqtUSWpPL+r0AFDBESbaYgQHg11ytdcDOvsvO8HO7w@mail.gmail.com>
 <CALte62z0211te6=5HdGWwd0ZkF3RnmM5s-UV0xVFMJhsG6MRUQ@mail.gmail.com>
 <CAMd5YszEPgTgGENo6Ek0-S4LrHi4KPa5=2xVo_TptP+=5o9e2g@mail.gmail.com>
 <CAJZFVeAGjtJtGN=HTUesCzDqVzzL18sZFoHwk_VKPC+FMrykvg@mail.gmail.com>
 <1509995885.2977722.1163515256.51D49E8A@webmail.messagingengine.com>
 <CALS7w+tdp4QxJgK19X1LJXtBjMNBQnPD6G7pXKn=JVaS07x3Cg@mail.gmail.com>
 <CAJZFVeARobwkW81fKsBVHKN0VGVdmTjmrJDUSTkq+uvTYpdVpA@mail.gmail.com>
 <CAMd5YszaMNfygXyfV6t_BK7yVGkrX6i=2Mp-NZOr-vm3T8zzJg@mail.gmail.com>
 <CAJZFVeDnhEL2wEptqB=ELPsf+1HB-HdQ19-N_wfpZwEWwCH7=A@mail.gmail.com>
 <CAMd5YswgEZOaQeAx++o7zbsCg+PHa7FzAuwkPoi4_5Su5Zp+dw@mail.gmail.com>
 <CAJZFVeCpk7aQsUkjX+caoGnL01=BRZbOgiTv8iDmxiAVBg4Z+A@mail.gmail.com>
 <CA+rPSbZaBXZUpf-EArRsNxGfQZv=wom09Gmt8A339qOy5QRO8Q@mail.gmail.com> <1513047241.516184.1201934328.324519C2@webmail.messagingengine.com>
In-Reply-To: <1513047241.516184.1201934328.324519C2@webmail.messagingengine.com>
From: Steven Aerts <steven.aerts@gmail.com>
Date: Tue, 12 Dec 2017 11:11:28 +0000
Message-ID: <CAJZFVeBChj2uq6dbDFyG5bFiOR32+nDiPMHCvjmC-bdnffFPQA@mail.gmail.com>
Subject: Re: [DISCUSS] KIP-218: Make KafkaFuture.Function java 8 lambda compatible
To: dev@kafka.apache.org
Content-Type: multipart/alternative; boundary="94eb2c191c6c0045f7056022b71c"
archived-at: Tue, 12 Dec 2017 11:11:52 -0000

--94eb2c191c6c0045f7056022b71c
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

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 <cmccabe@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=C3=A9aut=C3=A9 wrote:
> > Hi Steven,
> >
> > I noticed you are making KafkaFuture.addWaiter(...) public as part of
> > your
> > PR. This is a very useful method to add =E2=80=93 and you should mentio=
n it  in
> > the
> > KIP =E2=80=93 however addWaiter currently doesn't guard against excepti=
ons thrown
> > inside of the BiConsumer function, which is something we should probabl=
y
> > 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 you=
r
> > 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.aerts@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.bentley@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.aerts@gmail.com>
> > > wrote:
> > > >
> > > > > Hi Tom,
> > > > >
> > > > > I just made changes to the proposal of KIP-218, to make everythin=
g
> 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 ope=
n
> 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.bentley@gmail.com
> > > >:
> > > > >
> > > > > > Hi Steven,
> > > > > >
> > > > > > I'm particularly interested in seeing progress on this KIP as t=
he
> > > 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.aerts@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 name=
s
> with
> > > > > those
> > > > > > > of CompletableFuture?
> > > > > > >
> > > > > > > Thanks,
> > > > > > >
> > > > > > >
> > > > > > >    Steven
> > > > > > >
> > > > > > > Op vr 10 nov. 2017 om 12:12 schreef Ben Stopford <
> ben@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 <
> cmccabe@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 h=
as
> > > 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 functio=
ns
> > > > 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 goin=
g
> 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.bentley@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 <
> yuzhihong@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 <
> > > > > ismael@juma.me.uk>
> > > > > > > > > wrote:
> > > > > > > > > > >>
> > > > > > > > > > >> > On Wed, Nov 1, 2017 at 3:51 PM, Ted Yu <
> > > > yuzhihong@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
> > > > > > > > > > >> >
> > > > > > > > > > >>
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
>

--94eb2c191c6c0045f7056022b71c--