Dan Burkert has posted comments on this change.

Change subject: KUDU-1309: [java client] support tables with non-covering 
partition-key ranges
......................................................................


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/3388/4/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduClient.java:

Line 810:       } else {
> This will need a rebase + conflict resolution.
Done


Line 1152:       Map.Entry<byte[], byte[]> nonCoveredRange = 
getNonCoveredRange(tableId, key);
> Can use tableId here.
Done


http://gerrit.cloudera.org:8080/#/c/3388/4/java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java
File java/kudu-client/src/main/java/org/kududb/client/AsyncKuduSession.java:

PS4, Line 784: private Object tablet = null;
> Ugh. Can you add more color on why this approach was necessary? Why can't w
It could be done that way, but the downcasting would need to happen in the 
callback, so it's really just moving the ugliness.  I personally think having 
it wrapped up in this small class is good enough containment of the obviously 
poor abstraction.


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

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

Reply via email to