Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16276 )
Change subject: [KUDU-3177] Added kudu.snapshotTimestampMicros to kudu spark readOptions as optional property ...................................................................... Patch Set 3: (4 comments) Thank you for addressing all the feedback. Just a few small nits, otherwise it looks good to me. http://gerrit.cloudera.org:8080/#/c/16276/3//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16276/3//COMMIT_MSG@9 PS3, Line 9: Added property snapshotTimestampMs to spark read options which will allow consistant scans nit: wrap around 70-ish characters http://gerrit.cloudera.org:8080/#/c/16276/3/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala: http://gerrit.cloudera.org:8080/#/c/16276/3/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala@767 PS3, Line 767: assert Use assertTrue here and below. http://gerrit.cloudera.org:8080/#/c/16276/3/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/DefaultSourceTest.scala@804 PS3, Line 804: Thread.sleep(1000) Might be worth sleeping an extra second to avoid flakiness in the test. http://gerrit.cloudera.org:8080/#/c/16276/3/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala File java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala: http://gerrit.cloudera.org:8080/#/c/16276/3/java/kudu-spark/src/test/scala/org/apache/kudu/spark/kudu/KuduTestSuite.scala@35 PS3, Line 35: import org.apache.kudu.util.{CharUtil, DateUtil, DecimalUtil, HybridTimeUtil} nit: leave as separate import lines -- To view, visit http://gerrit.cloudera.org:8080/16276 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I00862c0e174a964efc6cab0b8141b1ac5a1bebc0 Gerrit-Change-Number: 16276 Gerrit-PatchSet: 3 Gerrit-Owner: Kevin McCarthy <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kevin McCarthy <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 04 Aug 2020 16:04:49 +0000 Gerrit-HasComments: Yes
