Todd Lipcon has posted comments on this change. Change subject: [java-client] separating Connection ......................................................................
Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/7146/10/java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java File java/kudu-client/src/main/java/org/apache/kudu/client/Connection.java: Line 84: */ > Could you add a comment to the class explaining why it's using a socket rea yea I consider the use of socket timeouts to be a bug rather than a feature... timeouts should be call-scoped rather than something on the socket level. but I dont think it's within scope of this patch to fix that. maybe a TODO? I thought we had a JIRA about this but I can't find it now - maybe JD knows. http://gerrit.cloudera.org:8080/#/c/7146/9/java/kudu-client/src/main/java/org/apache/kudu/client/RpcHelper.java File java/kudu-client/src/main/java/org/apache/kudu/client/RpcHelper.java: PS9, Line 57: RpcHelper nit: I have a bit of a pet peeve about classes named "Helper" because it's not very illustrative about what the class actually does. Maybe this is 'OutboundRpc' or 'RpcRouter' or 'RpcRetrier' or 'InFlightRpc' or something like that? (still have to review this patch in whole so maybe I'll have a better idea for a name after doing so!) -- To view, visit http://gerrit.cloudera.org:8080/7146 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Id4ac81d9454631e7501c31576c24f85e968bb871 Gerrit-PatchSet: 10 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
