Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18791 )
Change subject: KUDU-2671: Restore custom hash schemas properly ...................................................................... Patch Set 8: (4 comments) http://gerrit.cloudera.org:8080/#/c/18791/8/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java: http://gerrit.cloudera.org:8080/#/c/18791/8/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java@353 PS8, Line 353: @InterfaceAudience.Private : @InterfaceStability.Unstable : public List<Partition> getRangePartitionsWithTableHashSchema(long timeout) throws Exception { : return getRangePartitionsHelper(timeout, true); : } Do you mind adding a unit test for this new method just to make sure it works as expected and spot future regressions? http://gerrit.cloudera.org:8080/#/c/18791/8/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java@367 PS8, Line 367: @InterfaceAudience.Private : @InterfaceStability.Unstable Does this really provide any usage guidance for this private method? http://gerrit.cloudera.org:8080/#/c/18791/5/java/kudu-client/src/main/java/org/apache/kudu/client/RangePartition.java File java/kudu-client/src/main/java/org/apache/kudu/client/RangePartition.java: http://gerrit.cloudera.org:8080/#/c/18791/5/java/kudu-client/src/main/java/org/apache/kudu/client/RangePartition.java@29 PS5, Line 29: Limite > Changed to LimitedPrivate("Test), but I suppose I don't really understand t Right, that's just to inform users of the interface that even if a class is public, there are some restriction on whether it's safe using the API without risk of the API going away or changing: * https://yetus.apache.org/documentation/in-progress/javadocs/org/apache/yetus/audience/InterfaceAudience.html * https://github.com/apache/yetus/blob/main/audience-annotations-component/audience-annotations/src/main/java/org/apache/yetus/audience/InterfaceAudience.java http://gerrit.cloudera.org:8080/#/c/18791/8/java/kudu-client/src/main/java/org/apache/kudu/client/RangePartition.java File java/kudu-client/src/main/java/org/apache/kudu/client/RangePartition.java: http://gerrit.cloudera.org:8080/#/c/18791/8/java/kudu-client/src/main/java/org/apache/kudu/client/RangePartition.java@31 PS8, Line 31: public Can we get away by keeping this class package-private? At least, 'gradlew assemble' didn't report an error when I removed 'public' here. -- To view, visit http://gerrit.cloudera.org:8080/18791 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c28b306f2b630a609231a8fb2a5f5652b028d8e Gerrit-Change-Number: 18791 Gerrit-PatchSet: 8 Gerrit-Owner: Mahesh Reddy <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mahesh Reddy <[email protected]> Gerrit-Comment-Date: Wed, 03 Aug 2022 15:46:44 +0000 Gerrit-HasComments: Yes
