[GitHub] spark issue #19973: [SPARK-22779] FallbackConfigEntry's default value should...
Github user vanzin commented on the issue: https://github.com/apache/spark/pull/19973 @rxin we can close this now right? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19973: [SPARK-22779] FallbackConfigEntry's default value should...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19973 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84882/ Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19973: [SPARK-22779] FallbackConfigEntry's default value should...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19973 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19973: [SPARK-22779] FallbackConfigEntry's default value should...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19973 **[Test build #84882 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84882/testReport)** for PR 19973 at commit [`385c300`](https://github.com/apache/spark/commit/385c300c14a382654c2a1f94ccd2813487dbe26a). * This patch **fails Spark unit tests**. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19973: [SPARK-22779] FallbackConfigEntry's default value should...
Github user vanzin commented on the issue: https://github.com/apache/spark/pull/19973 Let me take a look... --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19973: [SPARK-22779] FallbackConfigEntry's default value should...
Github user rxin commented on the issue: https://github.com/apache/spark/pull/19973 @vanzin you got a min to submit a patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19973: [SPARK-22779] FallbackConfigEntry's default value should...
Github user vanzin commented on the issue:
https://github.com/apache/spark/pull/19973
Well, the default for fallback configs is the current value of the parent
conf - it needs context. The code you reference in the a previous comment has
that context (the SQL conf map), so the value of the parent conf might not be
the default.
If you look at `ConfigReader.getOrDefault` it has some (kinda nasty) code
to do this.
If I understand correctly, the problem is really here (the other
`getConfString` method):
```
def getConfString(key: String): String = {
Option(settings.get(key)).
orElse {
// Try to use the default value
Option(sqlConfEntries.get(key)).map(_.defaultValueString)
}.
getOrElse(throw new NoSuchElementException(key))
}
```
That will be broken for fallback configs with the existing code, but it
will also return the wrong thing with your updated code, in the case where the
parent conf is set.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19973: [SPARK-22779] FallbackConfigEntry's default value should...
Github user rxin commented on the issue: https://github.com/apache/spark/pull/19973 That's what the "default" is, isn't it? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
