Skip to content

Commit

Permalink
[SPARK-13727][CORE] SparkConf.contains does not consider deprecated keys
Browse files Browse the repository at this point in the history
The contains() method does not return consistently with get() if the key is deprecated. For example,
import org.apache.spark.SparkConf
val conf = new SparkConf()
conf.set("spark.io.compression.lz4.block.size", "12345")  # display some deprecated warning message
conf.get("spark.io.compression.lz4.block.size") # return 12345
conf.get("spark.io.compression.lz4.blockSize") # return 12345
conf.contains("spark.io.compression.lz4.block.size") # return true
conf.contains("spark.io.compression.lz4.blockSize") # return false

The fix will make the contains() and get() more consistent.

I've added a test case for this.

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
Unit tests should be sufficient.

Author: bomeng <bmeng@us.ibm.com>

Closes #11568 from bomeng/SPARK-13727.
  • Loading branch information
bomeng authored and Marcelo Vanzin committed Mar 10, 2016
1 parent d24801a commit 235f4ac
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 1 deletion.
5 changes: 4 additions & 1 deletion core/src/main/scala/org/apache/spark/SparkConf.scala
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,10 @@ class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging {
def getAppId: String = get("spark.app.id")

/** Does the configuration contain a given parameter? */
def contains(key: String): Boolean = settings.containsKey(key)
def contains(key: String): Boolean = {
settings.containsKey(key) ||
configsWithAlternatives.get(key).toSeq.flatten.exists { alt => contains(alt.key) }
}

/** Copy this object */
override def clone: SparkConf = {
Expand Down
14 changes: 14 additions & 0 deletions core/src/test/scala/org/apache/spark/SparkConfSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,20 @@ class SparkConfSuite extends SparkFunSuite with LocalSparkContext with ResetSyst
conf.set("spark.akka.lookupTimeout", "4")
assert(RpcUtils.lookupRpcTimeout(conf).duration === (4 seconds))
}

test("SPARK-13727") {
val conf = new SparkConf()
// set the conf in the deprecated way
conf.set("spark.io.compression.lz4.block.size", "12345")
// get the conf in the recommended way
assert(conf.get("spark.io.compression.lz4.blockSize") === "12345")
// we can still get the conf in the deprecated way
assert(conf.get("spark.io.compression.lz4.block.size") === "12345")
// the contains() also works as expected
assert(conf.contains("spark.io.compression.lz4.block.size"))
assert(conf.contains("spark.io.compression.lz4.blockSize"))
assert(conf.contains("spark.io.unknown") === false)
}
}

class Class1 {}
Expand Down

0 comments on commit 235f4ac

Please sign in to comment.