Jean-Daniel Cryans has posted comments on this change.

Change subject: [java client] AsyncKuduClient#delayedSendRpcToTablet should 
return a Deferred
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/4654/1//COMMIT_MSG
Commit Message:

PS1, Line 12: paths that simply handle background retries (don't
            : need a Deferred since we rely on KuduRpc#callback).
> Can we eliminate these? That is, can we rewrite that code to leverage the u
Mmmm possibly yes.


http://gerrit.cloudera.org:8080/#/c/4654/1/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:

PS1, Line 1060:     request.errback(e);
              :     LOG.debug("Cannot continue with this RPC: {} because of: 
{}", request, message, e);
              :     return Deferred.fromError(e);
> What's the point of both returning a Deferred and invoking request.errback(
Right, related to your comment on the commit message, if we just relied on the 
RPC's own Deferred, we wouldn't need to return one here and that'd be it.


Line 1261:   private <R> Deferred<R> delayedSendRpcToTablet(final KuduRpc<R> 
rpc, KuduException ex) {
> Even though it's private, you should document it to explain the semantic me
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/4654
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7c5e87b7dc3366ed9d72121a2421c5cd2304608b
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <d...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org>
Gerrit-HasComments: Yes

Reply via email to