Attila Bukor has posted comments on this change. Change subject: [java] KUDU-2103 Canonicalize hostnames in client ......................................................................
Patch Set 1: (5 comments) http://gerrit.cloudera.org:8080/#/c/7757/1/java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java File java/kudu-client/src/main/java/org/apache/kudu/client/ServerInfo.java: Line 74: public String getHostname() { > how about getAndCanonicalizeHostname()? that way since it has a verb in it deal http://gerrit.cloudera.org:8080/#/c/7757/1/java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java File java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java: Line 64: Method method = InetAddress.class.getDeclaredMethod("getCanonicalHostName"); > unused? yep, tried a different approach for this first, somehow forgot to delete it... PS1, Line 80: if (host.equals("master1.example.com") || host.equals("server123.example.com")) { : return new InetAddress[]{InetAddress.getByAddress(new byte[]{10, 1, 2, 3})}; : } > I think it's a bit messy to have this forward/reverse mapping hard-coded in good idea, thanks http://gerrit.cloudera.org:8080/#/c/7757/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestServerInfo.java: Line 41: public void testGetUnknownHostname() throws Exception { > can you add some comments here explaining what these test cases are asserti yep, will add Line 47: InetAddress ia = InetAddress.getByName("8.8.8.8"); > I don't understand the comment above. Can you make it a full sentence and d I think it's not actually necessary for my tests, I just copied it from the KUDU-1982 test case. Rewriting it. -- To view, visit http://gerrit.cloudera.org:8080/7757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I77204a185c1ab8e21cc833afb645543cbc0e340f Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Attila Bukor <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
