Pierre, those were good thoughts. Actually, I think you might have
unraveled some of the API for me by making this asserting that:
> My understanding is that the ResultSet is an abstraction over a list of
rows
> that may be fetched in background. ... It is not made to compose a
> chain of callbacks like CompletableFuture does, so it should not replace
it.
That thinking better separates the concerns of Client and ResultSet. I'll
think on this a bit and reply back with any revisions as a result for
further consideration.
Thanks
On Tue, Feb 16, 2016 at 7:47 AM, Pierre Laporte <pierre.lapo...@datastax.com
> wrote:
> Hi Stephen
>
> Overall, here are my impressions on your proposals:
>
> * -1 to have Client.submitAsync() return a ResultSet instead of a CF
> * +1 to have ResultSet.all() return a List<Row>
> * -1 to add a method ResultSet.allAsync() which returns a CF
> * +0 for the getInt -> asInt rename
>
> I think there is value in being async to the network stack. The typical
> idea
> for going async is to move the blocking or time consuming operations out of
> the
> way as fast as possible. To that extent I think it makes sense to have the
> entry point (Client.submit()) have an asynchronous version that returns a
> CompletableFuture.
>
> However, I am not sure it is useful to have a ResultSet.allAsync() method
> that
> would return a CF, if the entry point (the Client) already offers an
> asynchronous mode. The only use case it would address would be to be
> blocked
> during the network I/O send operation but not during the results retrieval.
>
> As for replacing CompletableFuture<ResultSet> with ResultSet, I am not sure
> it
> is a good idea. We would lose the chaining capabilities of
> CompletableFuture,
> like in the following:
>
> client.submitAsync(...).thenApply(MyClass::processResultSet, myExecutor);
>
> My understanding is that the ResultSet is an abstraction over a list of
> rows
> that may be fetched in background. It is allowed to block if none has yet
> been
> fetched. It is not made to compose a chain of callbacks like
> CompletableFuture
> does, so it should not replace it.
>
> --
>
> Pierre Laporte
> Performance Engineer
> www.datastax.com
>
>
> On 2016-02-13 15:40, Stephen Mallette <spmalle...@gmail.com> wrote:
> > I've been wanting to change something for some time now in Gremlin Driver
> ->
> > pretty much since GA, - but it's a breaking change and will make folks
> have>
> > to change up their code a bit so I've hesitated. Basically I want to
> change>
> > the submit() and submitAsync() API to no longer be async to the network>
> > stack. I think people find it confusing and it leads to some overly>
> > verbose. The change would turn signatures like this:>
> >
> > public ResultSet submit(final String gremlin)>
> > CompletableFuture<ResultSet> submitAsync(final String gremlin)>
> >
> > into this:>
> >
> > public List<Result> submit(final String gremlin)>
> > public ResultSet submitAsync(final String gremlin)>
> >
> > Under this new model, submit() blocks until the entire result is
> accounted>
> > for and submitAsync() blocks until the message hits the client side
> network>
> > stack and ResultSet behaves like a Future/Iterator as it does now.>
> >
> > So rather than:>
> >
> > Cluster cluster = Cluster.open();>
> > Client client = cluster.connect();>
> > int x = client.submit("1+1").all().get().get(0).getInt();>
> > int y = client.submitAsync("1+1").get().all().get().get(0).getInt();>
> >
> > we have:>
> >
> > Cluster cluster = Cluster.open();>
> > Client client = cluster.connect();>
> > int x = client.submit("1+1").get(0).getInt();>
> > int y = client.submitAsync("1+1").all().get().get(0).getInt();>
> >
> > While we haven't really reduced the amount of code significantly, a
> bunch>
> > of confusion is avoided here because submit() no longer requires the
> added>
> > call to all().get() to block until completion.>
> >
> > The example is a little dumb because it only returns one Result, in
> which>
> > case you would likely do:>
> >
> > int x = client.submit("1+1").get(0).getInt();>
> > int y = client.submitAsync("1+1").one().getInt();>
> >
> > Those two calls are functionally equivalent in terms of blocking.>
> > submitAsync() almost looks nicer in this case. That leads me back to>
> > all() which returns CompletableFuture<List<Result>>...perhaps that
> should>
> > change so that there is both all() and allAsync():>
> >
> > public List<Result> all()>
> > public CompletableFuture<List<Result>> allAsync()>
> >
> > In this case, the previous blocking example for all() simplifies to:>
> >
> > int y = client.submitAsync("1+1").all().get(0).getInt();>
> > int y = client.submitAsync("1+1").allAsync().get().get(0).getInt();>
> >
> > The same could be done for the some() method. I think the point is here
> to>
> > clearly mark for the users which methods are behaving async and which
> are>
> > going to block.>
> >
> > Finally, I think it would be better to deprecate the get* methods on
> Result>
> > in favor of as*. "as" seems to better describe what's happening when
> those>
> > methods on Result are called: type coercion and breaks up all the "get">
> > looking calls, thus:>
> >
> > int y = client.submitAsync("1+1").all().get(0).asInt();>
> > int y = client.submitAsync("1+1").allAsync().get().get(0).asInt();>
> >
> > I'd like to see this change in play for 3.2.0 if possible. Please let
> me>
> > know if there are any concerns or better ideas for this API. If there
> are>
> > no objections in the next 72 hours (Tuesday, February 16, 2016 at 9:30am>
> > EST), I'll assume lazy consensus and proceed forward.>
> >
> > Thanks,>
> >
> > Stephen>
> >
>
