Alexey Serbin has posted comments on this change. Change subject: java-client: improve error messages when failing to connect to secure cluster ......................................................................
Patch Set 2: (3 comments) Looks good but the new test failed when running by Jenkins http://gerrit.cloudera.org:8080/#/c/7824/2//COMMIT_MSG Commit Message: PS2, Line 11: unauthenticate nit: unauthenticated http://gerrit.cloudera.org:8080/#/c/7824/1/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: PS1, Line 395: LOG.error(message, e); : } : } > I think we are intentionally not logging this case because it was a frequen The idea was to explicitly say about the condition, so the message would not be treated as a red herring. Since the current version is not worse in that regard than the original one, I think it's fine to keep it as is. http://gerrit.cloudera.org:8080/#/c/7824/1/java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java File java/kudu-client/src/main/java/org/apache/kudu/client/Negotiator.java: PS1, Line 380: } : : if (chosenMech != null) { : return; : } : : // TODO(KUDU-1948): when the Java client has an option to require security, detect the case : // where the server is configured without Kerberos and the client requires it. : String message; : if (serverMechs.size() == 1 && serverMechs.contains("GSSAPI")) { : // Give a better error diagnostic for common case of an unauthenticated client connecting : // to a secure server. : message = "Server requires Kerberos, but this client is not authenticated (kinit)"; : } else { : > Done. I don't know if we have hard and fast rules about wrapping throws, b Yep, I also don't like the idea of splitting throws from exceptions. My point was that void xMethod(JavaStyleNameForAClass variableName) throws MySuperDuperSpecialException { ... } was easier to parse than void xMethod(JavaStyleNameForAClass variableName) throws MySuperDuperSpecialException { ... } especially when a method has several parameters. -- To view, visit http://gerrit.cloudera.org:8080/7824 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I41c3229dcda284dce57cb6f6930efe3b50fa9698 Gerrit-PatchSet: 2 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Jean-Daniel Cryans <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-HasComments: Yes
