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>
>
