Alexey Serbin has posted comments on this change. Change subject: [java] KUDU-2013 re-acquire authn token if expired ......................................................................
Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/7250/8/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: PS8, Line 276: Preconditions.checkState(state == State.NEGOTIATING); : Preconditions.checkState > true that assertions aren't on by default, so they are more like DCHECK() i Probably. My main motivation behind using Preconditions vs assert is because in some cases it's a situation which cannot be handled consistently if continuing further when a precondition is not true. Assert would just ignore that in non-debug run, and I'm not sure that's what we want. Line 482: return Channels.close(channel); > I'm not particularly concerned with the change, but generally would rather Agreed -- it's not a good idea to unintentionally 'hide' such small modifications in other changes. As I understand, the difference with disconnect() and close() is that closed channel cannot be re-used (i.e. open) again. And from the functionality point it seems both disconnect() and close() do the same in cast of TCP connections: https://stackoverflow.com/questions/14128910/netty-calling-channel-disconnect-actually-closes-the-channel -- To view, visit http://gerrit.cloudera.org:8080/7250 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I0be620629c9a8345ecd5e5679c80ee76ca4eaa57 Gerrit-PatchSet: 8 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: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: Will Berkeley <wdberke...@gmail.com> Gerrit-HasComments: Yes
