Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/10321 )
Change subject: KUDU-2403: Fix FakeDNS to work in Java 9+ ...................................................................... Patch Set 7: (7 comments) http://gerrit.cloudera.org:8080/#/c/10321/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10321/4//COMMIT_MSG@11 PS4, Line 11: implementation > implementation Done http://gerrit.cloudera.org:8080/#/c/10321/4//COMMIT_MSG@14 PS4, Line 14: implementation > same Done http://gerrit.cloudera.org:8080/#/c/10321/4//COMMIT_MSG@17 PS4, Line 17: NameServic > NameService Done http://gerrit.cloudera.org:8080/#/c/10321/4//COMMIT_MSG@19 PS4, Line 19: implementation > same Done http://gerrit.cloudera.org:8080/#/c/10321/4/java/gradle/tests.gradle File java/gradle/tests.gradle: http://gerrit.cloudera.org:8080/#/c/10321/4/java/gradle/tests.gradle@39 PS4, Line 39: if (JavaVersion.current().isJava9Compatible()) { > Could you reformat this a bit to reduce the duplication? Like, maybe a list Done http://gerrit.cloudera.org:8080/#/c/10321/7/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: http://gerrit.cloudera.org:8080/#/c/10321/7/java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java@145 PS7, Line 145: } catch (Exception e) { : throw new UnknownHostException(e.getMessage()); > wouldn't 'e' usually be an InvocationTargetException here, and then we woul The exceptions that could be thrown are NoSuchMethodException, IllegalAccessException, or InvocationTargetException. InvocationTargetException is likely to be the only one actually thrown unless some future java version breaks things. I think your suggested approach is a better way of handling this. I will also change this to only catch ReflectiveOperationException's given that covers all the expected exceptions. http://gerrit.cloudera.org:8080/#/c/10321/7/java/kudu-client/src/test/java/org/apache/kudu/client/FakeDNS.java@170 PS7, Line 170: throw new UnknownHostException(e.getMessage()); > same Done -- To view, visit http://gerrit.cloudera.org:8080/10321 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I319f42b6b878531325d6a48ce6ccdcc9b40fe2bc Gerrit-Change-Number: 10321 Gerrit-PatchSet: 7 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Mon, 07 May 2018 18:55:45 +0000 Gerrit-HasComments: Yes
