[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/21454 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191944805
--- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala ---
@@ -448,6 +473,22 @@ class SparkConf(loadDefaults: Boolean) extends
Cloneable with Logging with Seria
*/
private[spark] def getenv(name: String): String = System.getenv(name)
+ /**
+ * Wrapper method for get() methods which require some specific value
format. This catches
+ * any [[NumberFormatException]] or [[IllegalArgumentException]] and
re-raises it with the
+ * incorrectly configured key in the exception message.
+ */
+ private def catchIllegalValue[T](key: String)(getValue: => T): T = {
+try {
+ getValue
+} catch {
+ case e: NumberFormatException =>
+throw new NumberFormatException(s"Illegal value for config key
$key: ${e.getMessage}")
+ case e: IllegalArgumentException =>
+throw new IllegalArgumentException(s"Illegal value for config key
$key: ${e.getMessage}")
--- End diff --
ditto
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191944797
--- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala ---
@@ -448,6 +473,22 @@ class SparkConf(loadDefaults: Boolean) extends
Cloneable with Logging with Seria
*/
private[spark] def getenv(name: String): String = System.getenv(name)
+ /**
+ * Wrapper method for get() methods which require some specific value
format. This catches
+ * any [[NumberFormatException]] or [[IllegalArgumentException]] and
re-raises it with the
+ * incorrectly configured key in the exception message.
+ */
+ private def catchIllegalValue[T](key: String)(getValue: => T): T = {
+try {
+ getValue
+} catch {
+ case e: NumberFormatException =>
+throw new NumberFormatException(s"Illegal value for config key
$key: ${e.getMessage}")
--- End diff --
nit: please also add the cause like `throw new
NumberFormatException(s"Illegal value for config key $key: ${e.getMessage}", e)`
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191945182
--- Diff: core/src/test/scala/org/apache/spark/SparkConfSuite.scala ---
@@ -339,6 +340,38 @@ class SparkConfSuite extends SparkFunSuite with
LocalSparkContext with ResetSyst
}
}
+ val defaultIllegalValue = "SomeIllegalValue"
+ val illegalValueTests : Map[String, (SparkConf, String) => Any] = Map(
+"getTimeAsSeconds" -> (_.getTimeAsSeconds(_)),
+"getTimeAsSeconds with default" -> (_.getTimeAsSeconds(_,
defaultIllegalValue)),
+"getTimeAsMs" -> (_.getTimeAsMs(_)),
+"getTimeAsMs with default" -> (_.getTimeAsMs(_, defaultIllegalValue)),
+"getSizeAsBytes" -> (_.getSizeAsBytes(_)),
+"getSizeAsBytes with default string" -> (_.getSizeAsBytes(_,
defaultIllegalValue)),
+"getSizeAsBytes with default long" -> (_.getSizeAsBytes(_, 0L)),
+"getSizeAsKb" -> (_.getSizeAsKb(_)),
+"getSizeAsKb with default" -> (_.getSizeAsKb(_, defaultIllegalValue)),
+"getSizeAsMb" -> (_.getSizeAsMb(_)),
+"getSizeAsMb with default" -> (_.getSizeAsMb(_, defaultIllegalValue)),
+"getSizeAsGb" -> (_.getSizeAsGb(_)),
+"getSizeAsGb with default" -> (_.getSizeAsGb(_, defaultIllegalValue)),
+"getInt" -> (_.getInt(_, 0)),
+"getLong" -> (_.getLong(_, 0L)),
+"getDouble" -> (_.getDouble(_, 0.0)),
+"getBoolean" -> (_.getBoolean(_, false))
+ )
+
+ illegalValueTests.foreach { case (name, getValue) =>
+test(s"SPARK-24337: $name throws an useful error message with key
name") {
+ val key = "SomeKey"
+ val conf = new SparkConf()
+ conf.set(key, "SomeInvalidValue")
+ val thrown = intercept [IllegalArgumentException] {
--- End diff --
nit: remove the space between `intercept` and `[IllegalArgumentException]`
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191883353
--- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala ---
@@ -448,6 +473,20 @@ class SparkConf(loadDefaults: Boolean) extends
Cloneable with Logging with Seria
*/
private[spark] def getenv(name: String): String = System.getenv(name)
+ /**
+ * Wrapper method for get*() methods which require some specific value
format. This catches
+ * any [[NumberFormatException]] or [[IllegalArgumentException]] and
re-raises it with the
+ * incorrectly configured key in the exception message.
+ */
+ private def catchIllegalArgument[T](key: String)(getValue: => T): T = {
+try {
+ getValue
+} catch {
+ case e @ (_ : NumberFormatException | _ : IllegalArgumentException)
=>
+throw new IllegalArgumentException(s"Illegal value for config key
$key", e)
--- End diff --
- Use `s"Illegal value for config key $key: ${e.getMessage}"` so that the
error message of the cause can be easy to find.
- Avoid to change the exception type here. Let's just create the same
exception type for each case.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191899959
--- Diff: core/src/test/scala/org/apache/spark/SparkConfSuite.scala ---
@@ -339,6 +341,38 @@ class SparkConfSuite extends SparkFunSuite with
LocalSparkContext with ResetSyst
}
}
+ val defaultIllegalValue = "SomeIllegalValue"
+ val illegalValueTests : Map[String, (SparkConf, String) => Any] = Map(
+"getTimeAsSeconds" -> (_.getTimeAsSeconds(_)),
+"getTimeAsSeconds with default" -> (_.getTimeAsSeconds(_,
defaultIllegalValue)),
+"getTimeAsMs" -> (_.getTimeAsMs(_)),
+"getTimeAsMs with default" -> (_.getTimeAsMs(_, defaultIllegalValue)),
+"getSizeAsBytes" -> (_.getSizeAsBytes(_)),
+"getSizeAsBytes with default string" -> (_.getSizeAsBytes(_,
defaultIllegalValue)),
+"getSizeAsBytes with default long" -> (_.getSizeAsBytes(_, 0L)),
+"getSizeAsKb" -> (_.getSizeAsKb(_)),
+"getSizeAsKb with default" -> (_.getSizeAsKb(_, defaultIllegalValue)),
+"getSizeAsMb" -> (_.getSizeAsMb(_)),
+"getSizeAsMb with default" -> (_.getSizeAsMb(_, defaultIllegalValue)),
+"getSizeAsGb" -> (_.getSizeAsGb(_)),
+"getSizeAsGb with default" -> (_.getSizeAsGb(_, defaultIllegalValue)),
+"getInt" -> (_.getInt(_, 0)),
+"getLong" -> (_.getLong(_, 0L)),
+"getDouble" -> (_.getDouble(_, 0.0)),
+"getBoolean" -> (_.getBoolean(_, false))
+ )
+
+ illegalValueTests.foreach { case (name, getValue) =>
+test(s"SPARK-24337: $name throws an useful error message with key
name") {
+ val key = "SomeKey"
+ val conf = new SparkConf()
+ conf.set(key, "SomeInvalidValue")
+ val thrown = the [IllegalArgumentException] thrownBy {
+getValue(conf, key)
+ }
+ thrown.getMessage should include (key)
--- End diff --
@PenguinToast something like this
```
val thrown = intercept[IllegalArgumentException] {
getValue(conf, key)
}
assert(thrown.getMessage.contains(key))
```
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user PenguinToast commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191849201
--- Diff: core/src/test/scala/org/apache/spark/SparkConfSuite.scala ---
@@ -339,6 +341,38 @@ class SparkConfSuite extends SparkFunSuite with
LocalSparkContext with ResetSyst
}
}
+ val defaultIllegalValue = "SomeIllegalValue"
+ val illegalValueTests : Map[String, (SparkConf, String) => Any] = Map(
+"getTimeAsSeconds" -> (_.getTimeAsSeconds(_)),
+"getTimeAsSeconds with default" -> (_.getTimeAsSeconds(_,
defaultIllegalValue)),
+"getTimeAsMs" -> (_.getTimeAsMs(_)),
+"getTimeAsMs with default" -> (_.getTimeAsMs(_, defaultIllegalValue)),
+"getSizeAsBytes" -> (_.getSizeAsBytes(_)),
+"getSizeAsBytes with default string" -> (_.getSizeAsBytes(_,
defaultIllegalValue)),
+"getSizeAsBytes with default long" -> (_.getSizeAsBytes(_, 0L)),
+"getSizeAsKb" -> (_.getSizeAsKb(_)),
+"getSizeAsKb with default" -> (_.getSizeAsKb(_, defaultIllegalValue)),
+"getSizeAsMb" -> (_.getSizeAsMb(_)),
+"getSizeAsMb with default" -> (_.getSizeAsMb(_, defaultIllegalValue)),
+"getSizeAsGb" -> (_.getSizeAsGb(_)),
+"getSizeAsGb with default" -> (_.getSizeAsGb(_, defaultIllegalValue)),
+"getInt" -> (_.getInt(_, 0)),
+"getLong" -> (_.getLong(_, 0L)),
+"getDouble" -> (_.getDouble(_, 0.0)),
+"getBoolean" -> (_.getBoolean(_, false))
+ )
+
+ illegalValueTests.foreach { case (name, getValue) =>
+test(s"SPARK-24337: $name throws an useful error message with key
name") {
+ val key = "SomeKey"
+ val conf = new SparkConf()
+ conf.set(key, "SomeInvalidValue")
+ val thrown = the [IllegalArgumentException] thrownBy {
+getValue(conf, key)
+ }
+ thrown.getMessage should include (key)
--- End diff --
Sure. Should we change the above block to a try ... catch, or leave it as
is? I'm not sure on the recommended style here.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191626037
--- Diff: core/src/test/scala/org/apache/spark/SparkConfSuite.scala ---
@@ -25,14 +25,16 @@ import scala.language.postfixOps
import scala.util.{Random, Try}
import com.esotericsoftware.kryo.Kryo
+import org.scalatest.Matchers
import org.apache.spark.deploy.history.config._
import org.apache.spark.internal.config._
import org.apache.spark.network.util.ByteUnit
import org.apache.spark.serializer.{JavaSerializer, KryoRegistrator,
KryoSerializer}
import org.apache.spark.util.{ResetSystemProperties, RpcUtils}
-class SparkConfSuite extends SparkFunSuite with LocalSparkContext with
ResetSystemProperties {
+class SparkConfSuite extends SparkFunSuite with LocalSparkContext with
ResetSystemProperties with
+ Matchers {
--- End diff --
I would do `with Matchers` here.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191625768
--- Diff: core/src/test/scala/org/apache/spark/SparkConfSuite.scala ---
@@ -339,6 +341,38 @@ class SparkConfSuite extends SparkFunSuite with
LocalSparkContext with ResetSyst
}
}
+ val defaultIllegalValue = "SomeIllegalValue"
+ val illegalValueTests : Map[String, (SparkConf, String) => Any] = Map(
+"getTimeAsSeconds" -> (_.getTimeAsSeconds(_)),
+"getTimeAsSeconds with default" -> (_.getTimeAsSeconds(_,
defaultIllegalValue)),
+"getTimeAsMs" -> (_.getTimeAsMs(_)),
+"getTimeAsMs with default" -> (_.getTimeAsMs(_, defaultIllegalValue)),
+"getSizeAsBytes" -> (_.getSizeAsBytes(_)),
+"getSizeAsBytes with default string" -> (_.getSizeAsBytes(_,
defaultIllegalValue)),
+"getSizeAsBytes with default long" -> (_.getSizeAsBytes(_, 0L)),
+"getSizeAsKb" -> (_.getSizeAsKb(_)),
+"getSizeAsKb with default" -> (_.getSizeAsKb(_, defaultIllegalValue)),
+"getSizeAsMb" -> (_.getSizeAsMb(_)),
+"getSizeAsMb with default" -> (_.getSizeAsMb(_, defaultIllegalValue)),
+"getSizeAsGb" -> (_.getSizeAsGb(_)),
+"getSizeAsGb with default" -> (_.getSizeAsGb(_, defaultIllegalValue)),
+"getInt" -> (_.getInt(_, 0)),
+"getLong" -> (_.getLong(_, 0L)),
+"getDouble" -> (_.getDouble(_, 0.0)),
+"getBoolean" -> (_.getBoolean(_, false))
+ )
+
+ illegalValueTests.foreach { case (name, getValue) =>
+test(s"SPARK-24337: $name throws an useful error message with key
name") {
+ val key = "SomeKey"
+ val conf = new SparkConf()
+ conf.set(key, "SomeInvalidValue")
+ val thrown = the [IllegalArgumentException] thrownBy {
+getValue(conf, key)
+ }
+ thrown.getMessage should include (key)
--- End diff --
Shall we stick to `assert` syntax?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user HyukjinKwon commented on a diff in the pull request: https://github.com/apache/spark/pull/21454#discussion_r191625705 --- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala --- @@ -448,6 +473,20 @@ class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging with Seria */ private[spark] def getenv(name: String): String = System.getenv(name) + /** + * Wrapper method for get*() methods which require some specific value format. This catches --- End diff -- `get*()` .. ? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user HyukjinKwon commented on a diff in the pull request: https://github.com/apache/spark/pull/21454#discussion_r191625505 --- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala --- @@ -265,108 +265,121 @@ class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging with Seria * Get a time parameter as seconds; throws a NoSuchElementException if it's not set. If no * suffix is provided then seconds are assumed. * @throws java.util.NoSuchElementException If the time parameter is not set + * @throws IllegalArgumentException If the value can't be interpreted as seconds --- End diff -- I usually avoid abbreviation in the documentation though. `can't` -> `cannot`. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user jiangxb1987 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191584812
--- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala ---
@@ -448,6 +473,20 @@ class SparkConf(loadDefaults: Boolean) extends
Cloneable with Logging with Seria
*/
private[spark] def getenv(name: String): String = System.getenv(name)
+ /**
+ * Wrapper method for get*() methods which require some specific value
format. This catches
+ * any [[NumberFormatException]] or [[IllegalArgumentException]] and
re-raises it with the
+ * incorrectly configured key in the exception message.
+ */
+ private def catchIllegalArgument[T](key: String)(getValue: => T): T = {
--- End diff --
According to what it actually does `catchIllegalArgument` don't seems to be
a great name for this function, maybe `catchIllegalValue`?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user jiangxb1987 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191582665
--- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala ---
@@ -394,23 +407,35 @@ class SparkConf(loadDefaults: Boolean) extends
Cloneable with Logging with Seria
}
- /** Get a parameter as an integer, falling back to a default if not set
*/
- def getInt(key: String, defaultValue: Int): Int = {
+ /**
+ * Get a parameter as an integer, falling back to a default if not set
+ * @throws IllegalArgumentException If the value can't be interpreted as
an integer
+ */
+ def getInt(key: String, defaultValue: Int): Int =
catchIllegalArgument(key) {
getOption(key).map(_.toInt).getOrElse(defaultValue)
}
- /** Get a parameter as a long, falling back to a default if not set */
- def getLong(key: String, defaultValue: Long): Long = {
+ /**
+ * Get a parameter as a long, falling back to a default if not set
+ * @throws IllegalArgumentException If the value can't be interpreted as
an long
+ */
+ def getLong(key: String, defaultValue: Long): Long =
catchIllegalArgument(key) {
getOption(key).map(_.toLong).getOrElse(defaultValue)
}
- /** Get a parameter as a double, falling back to a default if not set */
- def getDouble(key: String, defaultValue: Double): Double = {
+ /**
+ * Get a parameter as a double, falling back to a default if not ste
+ * @throws IllegalArgumentException If the value can't be interpreted as
an double
+ */
+ def getDouble(key: String, defaultValue: Double): Double =
catchIllegalArgument(key) {
getOption(key).map(_.toDouble).getOrElse(defaultValue)
}
- /** Get a parameter as a boolean, falling back to a default if not set */
- def getBoolean(key: String, defaultValue: Boolean): Boolean = {
+ /**
+ * Get a parameter as a boolean, falling back to a default if not set
+ * @throws IllegalArgumentException If the value can't be interpreted as
an boolean
--- End diff --
nit: `an boolean` -> `a boolean`
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user jiangxb1987 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191582611
--- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala ---
@@ -394,23 +407,35 @@ class SparkConf(loadDefaults: Boolean) extends
Cloneable with Logging with Seria
}
- /** Get a parameter as an integer, falling back to a default if not set
*/
- def getInt(key: String, defaultValue: Int): Int = {
+ /**
+ * Get a parameter as an integer, falling back to a default if not set
+ * @throws IllegalArgumentException If the value can't be interpreted as
an integer
+ */
+ def getInt(key: String, defaultValue: Int): Int =
catchIllegalArgument(key) {
getOption(key).map(_.toInt).getOrElse(defaultValue)
}
- /** Get a parameter as a long, falling back to a default if not set */
- def getLong(key: String, defaultValue: Long): Long = {
+ /**
+ * Get a parameter as a long, falling back to a default if not set
+ * @throws IllegalArgumentException If the value can't be interpreted as
an long
+ */
+ def getLong(key: String, defaultValue: Long): Long =
catchIllegalArgument(key) {
getOption(key).map(_.toLong).getOrElse(defaultValue)
}
- /** Get a parameter as a double, falling back to a default if not set */
- def getDouble(key: String, defaultValue: Double): Double = {
+ /**
+ * Get a parameter as a double, falling back to a default if not ste
+ * @throws IllegalArgumentException If the value can't be interpreted as
an double
--- End diff --
nit: `an double` -> `a double`
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
Github user jiangxb1987 commented on a diff in the pull request:
https://github.com/apache/spark/pull/21454#discussion_r191582499
--- Diff: core/src/main/scala/org/apache/spark/SparkConf.scala ---
@@ -394,23 +407,35 @@ class SparkConf(loadDefaults: Boolean) extends
Cloneable with Logging with Seria
}
- /** Get a parameter as an integer, falling back to a default if not set
*/
- def getInt(key: String, defaultValue: Int): Int = {
+ /**
+ * Get a parameter as an integer, falling back to a default if not set
+ * @throws IllegalArgumentException If the value can't be interpreted as
an integer
+ */
+ def getInt(key: String, defaultValue: Int): Int =
catchIllegalArgument(key) {
getOption(key).map(_.toInt).getOrElse(defaultValue)
}
- /** Get a parameter as a long, falling back to a default if not set */
- def getLong(key: String, defaultValue: Long): Long = {
+ /**
+ * Get a parameter as a long, falling back to a default if not set
+ * @throws IllegalArgumentException If the value can't be interpreted as
an long
--- End diff --
nit: `an long` -> `a long`
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #21454: [SPARK-24337][Core] Improve error messages for Sp...
GitHub user PenguinToast opened a pull request: https://github.com/apache/spark/pull/21454 [SPARK-24337][Core] Improve error messages for Spark conf values ## What changes were proposed in this pull request? Improve the exception messages when retrieving Spark conf values to include the key name when the value is invalid. ## How was this patch tested? Unit tests for all get* operations in SparkConf that require a specific value format You can merge this pull request into a Git repository by running: $ git pull https://github.com/PenguinToast/spark SPARK-24337-spark-config-errors Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/21454.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #21454 commit f198f28b1a3d7380a09e5687438a264101cc6965 Author: William Sheu Date: 2018-05-29T18:35:25Z [SPARK-24337][Core] Improve error messages for Spark conf values --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
