Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/10839 )
Change subject: spark: Expose socketReadTimeoutMs to Spark connector ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/10839/3/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala: http://gerrit.cloudera.org:8080/#/c/10839/3/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/DefaultSource.scala@188 PS3, Line 188: socketReadTimeoutMs match { : case Some(_) => context.socketReadTimeoutMs = socketReadTimeoutMs : case None => : } > simplify this to just ah, fair point :) http://gerrit.cloudera.org:8080/#/c/10839/3/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala File java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala: http://gerrit.cloudera.org:8080/#/c/10839/3/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@91 PS3, Line 91: var socketReadTimeoutMs: Option[Long] = None > 'var' is wrong here -- if the caller gets the asyncClient, changes the read Oh I didn't notice that asyncClient is a lazy val. Done, except I just implemented it as an auxiliary constructor. http://gerrit.cloudera.org:8080/#/c/10839/3/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@111 PS3, Line 111: import kudu.KuduContext._ > What is this for? I'd remove if possible, underscore imports are non-explic It was already there but I've removed it. http://gerrit.cloudera.org:8080/#/c/10839/3/java/kudu-spark/src/main/scala/org/apache/kudu/spark/kudu/KuduContext.scala@399 PS3, Line 399: @InterfaceAudience.Private > These aren't necessary since it's a private object. Done -- To view, visit http://gerrit.cloudera.org:8080/10839 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0ab0ff0b242790caffb7e2848958148ffe547c4d Gerrit-Change-Number: 10839 Gerrit-PatchSet: 3 Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Comment-Date: Thu, 12 Jul 2018 23:44:23 +0000 Gerrit-HasComments: Yes
