Andrey,
I see. So in a nutshell, you're saying that we want to return a future that
the user's code is not allowed to complete. In this case, I think it's
clear that CompletableFuture is not what we need. We actually need a
NonCompletableFuture :)
My vote is for the custom interface.
-Val
On Fri, Mar 26, 2021 at 2:25 AM Andrey Mashenkov <andrey.mashen...@gmail.com>
wrote:
> Val,
>
> The methods below shouldn't be accessible for user:
> complete()
> completeExceptionaly()
>
> Returning CompletableFuture we must always make a copy to prevent the
> original future from being completed by mistake.
> I think it will NOT be enough to do that returing the future to the
> end-user, but from every critical module to the outside of the module,
> e.g. to plugins. The impact of disclosing ExchangeFuture,
> PartitionReleaseFuture to plugins may be serious.
>
> IgniteFuture<T> extends Future<T>, CompletionStage<T> which implementation
> will just wrap CompletableFuture these issues will be resolved in natural
> way.
> In addition we can force toCompletableFuture() method to return a defensive
> copy(), that resolves the last concern.
>
>
> On Fri, Mar 26, 2021 at 11:38 AM Konstantin Orlov <kor...@gridgain.com>
> wrote:
>
> > CompletableFuture seems a better option to me.
> >
> > --
> > Regards,
> > Konstantin Orlov
> >
> >
> >
> >
> > > On 26 Mar 2021, at 11:07, Pavel Tupitsyn <ptupit...@apache.org> wrote:
> > >
> > > On the one hand, I agree with Alexey.
> > > CompletableFuture has complete* methods which should not be available
> to
> > > the user code.
> > > This can be solved with a simple interface like we do in Thin Client:
> > > IgniteClientFuture<T> extends Future<T>, CompletionStage<T>
> > >
> > >
> > > On the other hand:
> > > - CompletionStage has toCompletableFuture anyway (rather weird)
> > > - Other libraries use CompletableFuture and it seems to be fine
> > > - Using CompletableFuture is the simplest approach
> > >
> > >
> > > So I lean towards CompletableFuture too.
> > >
> > > On Fri, Mar 26, 2021 at 10:46 AM Alexey Kukushkin <
> > kukushkinale...@gmail.com>
> > > wrote:
> > >
> > >> I do not like Java's CompletableFuture and prefer our own Future
> > (revised
> > >> IgniteFuture).
> > >>
> > >> My understanding of the Future (or Promise) pattern in general is
> having
> > >> two separate APIs:
> > >>
> > >> 1. Server-side: create, set result, raise error, cancel from server.
> > >> 2. Client-side: get result, handle error, cancel from client
> > >>
> > >> Java's CompletableFuture looks like both the client-side and
> > >> server-side API. The "Completeable" prefix in the name is already
> > confusing
> > >> for a client since it cannot "complete" an operation, only a server
> can.
> > >>
> > >> I would create our own IgniteFuture adding client-side functionality
> we
> > >> currently miss (like client-side cancellation).
> > >>
> > >>
> > >> пт, 26 мар. 2021 г. в 01:08, Valentin Kulichenko <
> > >> valentin.kuliche...@gmail.com>:
> > >>
> > >>> Andrey,
> > >>>
> > >>> Can you compile a full list of these risky methods, and elaborate on
> > what
> > >>> the risks are?
> > >>>
> > >>> Generally, CompletableFuture is a much better option, because it's
> > >>> standard. But we need to make sure it actually fits our needs and
> > doesn't
> > >>> do more harm than good.
> > >>>
> > >>> -Val
> > >>>
> > >>> On Thu, Mar 25, 2021 at 12:23 PM Alexei Scherbakov <
> > >>> alexey.scherbak...@gmail.com> wrote:
> > >>>
> > >>>> I think both options are fine, but personally lean toward
> > >>>> CompletableFuture.
> > >>>>
> > >>>> чт, 25 мар. 2021 г. в 17:56, Atri Sharma <a...@apache.org>:
> > >>>>
> > >>>>> I would suggest using CompletableFuture -- I don't see a need for a
> > >>>> custom
> > >>>>> interface that is unique to us.
> > >>>>>
> > >>>>> It also allows a lower barrier for new contributors for
> understanding
> > >>>>> existing code
> > >>>>>
> > >>>>> On Thu, 25 Mar 2021, 20:18 Andrey Mashenkov, <
> > >>> andrey.mashen...@gmail.com
> > >>>>>
> > >>>>> wrote:
> > >>>>>
> > >>>>>> Hi Igniters,
> > >>>>>>
> > >>>>>> I'd like to start a discussion about replacing our custom
> > >>> IgniteFuture
> > >>>>>> class with CompletableFuture - existed JDK class
> > >>>>>> or rework it's implementation (like some other products done) to a
> > >>>>>> composition of CompletionStage and Future interfaces.
> > >>>>>> or maybe other option if you have any ideas. Do you?
> > >>>>>>
> > >>>>>> 1. The first approach pros and cons are
> > >>>>>> + Well-known JDK class
> > >>>>>> + Already implemented
> > >>>>>> - It is a class, not an interface.
> > >>>>>> - Expose some potentially harmful methods like "complete()".
> > >>>>>>
> > >>>>>> On the other side, it has copy() method to create defensive copy
> > >> and
> > >>>>>> minimalCompletionStage() to restrict harmful method usage.
> > >>>>>> Thus, this look like an applicable solution, but we should be
> > >> careful
> > >>>>>> exposing internal future to the outside.
> > >>>>>>
> > >>>>>> 2. The second approach is to implement our own interface like the
> > >>> next
> > >>>>> one:
> > >>>>>>
> > >>>>>> interface IgniteFuture<T> extends CompletableStage<T>, Future<T> {
> > >>>>>> }
> > >>>>>>
> > >>>>>> Pros and cons are
> > >>>>>> + Our interfaces/classes contracts will expose an interface rather
> > >>> than
> > >>>>>> concrete implementation.
> > >>>>>> + All methods are safe.
> > >>>>>> - Some implementation is required.
> > >>>>>> - CompletableStage has a method toCompletableFuture() and can be
> > >>>>> converted
> > >>>>>> to CompletableFuture. This should be supported.
> > >>>>>>
> > >>>>>> However, we still could wrap CompletableFuture and don't bother
> > >> about
> > >>>>>> creating a defensive copy.
> > >>>>>>
> > >>>>>>
> > >>>>>> Other project experience:
> > >>>>>> * Spotify uses CompletableFuture directly [1].
> > >>>>>> * Redis goes the second approach [2]
> > >>>>>> * Vertx explicitly extends CompletableFuture [3]. However, they
> > >> have
> > >>>>> custom
> > >>>>>> future classes and a number of helpers that could be replaced with
> > >>>>>> CompletableStage. Maybe it is just a legacy.'
> > >>>>>>
> > >>>>>> Any thoughts?
> > >>>>>>
> > >>>>>> [1]
> > >>>>>>
> > >>>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> https://spotify.github.io/completable-futures/apidocs/com/spotify/futures/ConcurrencyReducer.html
> > >>>>>> [2]
> > >>>>>>
> > >>>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> https://lettuce.io/lettuce-4/release/api/com/lambdaworks/redis/RedisFuture.html
> > >>>>>> [3]
> > >>>>>>
> > >>>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>
> >
> https://javadoc.io/static/org.jspare.vertx/vertx-jspare/1.1.0-M03/org/jspare/vertx/concurrent/VertxCompletableFuture.html
> > >>>>>> --
> > >>>>>> Best regards,
> > >>>>>> Andrey V. Mashenkov
> > >>>>>>
> > >>>>>
> > >>>>
> > >>>>
> > >>>> --
> > >>>>
> > >>>> Best regards,
> > >>>> Alexei Scherbakov
> > >>>>
> > >>>
> > >>
> > >>
> > >> --
> > >> Best regards,
> > >> Alexey
> > >>
> >
> >
>
> --
> Best regards,
> Andrey V. Mashenkov
>
