[KYUUBI #3659] Support alternative keys in ConfigBuilder

### _Why are the changes needed?_ Refer Spark PR: apache/spark#18110 ConfigBuilder builds ConfigEntry which can only read value with one key, if we wanna change the config name but still keep the old one, it's hard to do. This PR introduce ConfigBuilder.withAlternative, to support reading config value with alternative keys. ### _How was this patch tested?_ - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [ ] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #3659 from turboFei/conf_alternative. Closes #3659 e268fef [Fei Wang] add ut a2300b2 [Fei Wang] add ut 53eccf9 [Fei Wang] Support alternative keys in ConfigBuilder Authored-by: Fei Wang <fwang12@ebay.com> Signed-off-by: Fei Wang <fwang12@ebay.com>
apache · Oct 18, 2022 · a6832b7 · a6832b7
1 parent 84297ea
commit a6832b7
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 3 deletions.
diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigBuilder.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigBuilder.scala
@@ -30,6 +30,7 @@ private[kyuubi] case class ConfigBuilder(key: String) {
   private[config] var _onCreate: Option[ConfigEntry[_] => Unit] = None
   private[config] var _type = ""
   private[config] var _internal = false
+  private[config] var _alternatives = List.empty[String]
 
   def internal: ConfigBuilder = {
     _internal = true
@@ -51,6 +52,11 @@ private[kyuubi] case class ConfigBuilder(key: String) {
     this
   }
 
+  def withAlternative(key: String): ConfigBuilder = {
+    _alternatives = _alternatives :+ key
+    this
+  }
+
   private def toNumber[T](s: String, converter: String => T): T = {
     try {
       converter(s.trim)
@@ -117,7 +123,7 @@ private[kyuubi] case class ConfigBuilder(key: String) {
 
   def fallbackConf[T](fallback: ConfigEntry[T]): ConfigEntry[T] = {
     val entry =
-      new ConfigEntryFallback[T](key, _doc, _version, _internal, fallback)
+      new ConfigEntryFallback[T](key, _alternatives, _doc, _version, _internal, fallback)
     _onCreate.foreach(_(entry))
     entry
   }
@@ -177,6 +183,7 @@ private[kyuubi] case class TypedConfigBuilder[T](
   def createOptional: OptionalConfigEntry[T] = {
     val entry = new OptionalConfigEntry(
       parent.key,
+      parent._alternatives,
       fromStr,
       toStr,
       parent._doc,
@@ -193,6 +200,7 @@ private[kyuubi] case class TypedConfigBuilder[T](
       val d = fromStr(toStr(default))
       val entry = new ConfigEntryWithDefault(
         parent.key,
+        parent._alternatives,
         d,
         fromStr,
         toStr,
@@ -207,6 +215,7 @@ private[kyuubi] case class TypedConfigBuilder[T](
   def createWithDefaultString(default: String): ConfigEntryWithDefaultString[T] = {
     val entry = new ConfigEntryWithDefaultString(
       parent.key,
+      parent._alternatives,
       default,
       fromStr,
       toStr,

diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigEntry.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigEntry.scala
@@ -19,6 +19,7 @@ package org.apache.kyuubi.config
 
 trait ConfigEntry[T] {
   def key: String
+  def alternatives: List[String]
   def valueConverter: String => T
   def strConverter: T => String
   def doc: String
@@ -34,7 +35,7 @@ trait ConfigEntry[T] {
   }
 
   final protected def readString(provider: ConfigProvider): Option[String] = {
-    provider.get(key)
+    alternatives.foldLeft(provider.get(key))((res, nextKey) => res.orElse(provider.get(nextKey)))
   }
 
   def readFrom(conf: ConfigProvider): T
@@ -44,6 +45,7 @@ trait ConfigEntry[T] {
 
 class OptionalConfigEntry[T](
     _key: String,
+    _alternatives: List[String],
     rawValueConverter: String => T,
     rawStrConverter: T => String,
     _doc: String,
@@ -70,6 +72,8 @@ class OptionalConfigEntry[T](
 
   override def key: String = _key
 
+  override def alternatives: List[String] = _alternatives
+
   override def doc: String = _doc
 
   override def version: String = _version
@@ -81,6 +85,7 @@ class OptionalConfigEntry[T](
 
 class ConfigEntryWithDefault[T](
     _key: String,
+    _alternatives: List[String],
     _defaultVal: T,
     _valueConverter: String => T,
     _strConverter: T => String,
@@ -98,6 +103,8 @@ class ConfigEntryWithDefault[T](
 
   override def key: String = _key
 
+  override def alternatives: List[String] = _alternatives
+
   override def valueConverter: String => T = _valueConverter
 
   override def strConverter: T => String = _strConverter
@@ -113,6 +120,7 @@ class ConfigEntryWithDefault[T](
 
 class ConfigEntryWithDefaultString[T](
     _key: String,
+    _alternatives: List[String],
     _defaultVal: String,
     _valueConverter: String => T,
     _strConverter: T => String,
@@ -131,6 +139,8 @@ class ConfigEntryWithDefaultString[T](
 
   override def key: String = _key
 
+  override def alternatives: List[String] = _alternatives
+
   override def valueConverter: String => T = _valueConverter
 
   override def strConverter: T => String = _strConverter
@@ -146,6 +156,7 @@ class ConfigEntryWithDefaultString[T](
 
 class ConfigEntryFallback[T](
     _key: String,
+    _alternatives: List[String],
     _doc: String,
     _version: String,
     _internal: Boolean,
@@ -160,6 +171,8 @@ class ConfigEntryFallback[T](
 
   override def key: String = _key
 
+  override def alternatives: List[String] = _alternatives
+
   override def valueConverter: String => T = fallback.valueConverter
 
   override def strConverter: T => String = fallback.strConverter

diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigEntrySuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigEntrySuite.scala
@@ -25,6 +25,7 @@ class ConfigEntrySuite extends KyuubiFunSuite {
     val doc = "this is dummy documentation"
     val e1 = new OptionalConfigEntry[Int](
       "kyuubi.int.spark",
+      List.empty[String],
       s => s.toInt + 1,
       v => (v - 1).toString,
       doc,
@@ -49,6 +50,7 @@ class ConfigEntrySuite extends KyuubiFunSuite {
     assert(conf.get(e1).isEmpty)
     val e = intercept[IllegalArgumentException](new OptionalConfigEntry[Int](
       "kyuubi.int.spark",
+      List.empty[String],
       s => s.toInt + 1,
       v => (v - 1).toString,
       "this is dummy documentation",
@@ -65,6 +67,7 @@ class ConfigEntrySuite extends KyuubiFunSuite {
   test("config entry with default") {
     val e1 = new ConfigEntryWithDefault[Long](
       "kyuubi.long.spark",
+      List.empty[String],
       2,
       s => s.toLong + 1,
       v => (v - 1).toString,
@@ -95,6 +98,7 @@ class ConfigEntrySuite extends KyuubiFunSuite {
   test("config entry with default string") {
     val e1 = new ConfigEntryWithDefaultString[Double](
       "kyuubi.double.spark",
+      List.empty[String],
       "3.0",
       s => java.lang.Double.valueOf(s),
       v => v.toString,
@@ -127,7 +131,13 @@ class ConfigEntrySuite extends KyuubiFunSuite {
       .version("1.1.1")
       .stringConf.createWithDefault("origin")
     val fallback =
-      new ConfigEntryFallback[String]("kyuubi.fallback.spark", "fallback", "1.2.0", false, origin)
+      new ConfigEntryFallback[String](
+        "kyuubi.fallback.spark",
+        List.empty[String],
+        "fallback",
+        "1.2.0",
+        false,
+        origin)
 
     assert(fallback.key === "kyuubi.fallback.spark")
     assert(fallback.valueConverter("2") === "2")
@@ -151,6 +161,7 @@ class ConfigEntrySuite extends KyuubiFunSuite {
   test("An unregistered config should not be get") {
     val config = new ConfigEntryWithDefaultString[Double](
       "kyuubi.unregistered.spark",
+      List.empty[String],
       "3.0",
       s => java.lang.Double.valueOf(s),
       v => v.toString,
@@ -171,4 +182,27 @@ class ConfigEntrySuite extends KyuubiFunSuite {
       "doc=doc, version=, type=double) is not registered"))
   }
 
+  test("support alternative keys in ConfigBuilder") {
+    val config = new ConfigEntryWithDefaultString[Double](
+      "kyuubi.key",
+      List("kyuubi.key.alternative", "kyuubi.key.alternative2"),
+      "3.0",
+      s => java.lang.Double.valueOf(s),
+      v => v.toString,
+      "doc",
+      "",
+      "double",
+      false)
+
+    val conf = KyuubiConf()
+    KyuubiConf.register(config)
+    assert(conf.get(config) == 3.0)
+    conf.set("kyuubi.key.alternative", "4.0")
+    conf.set("kyuubi.key.alternative2", "5.0")
+    assert(conf.get(config) == 4.0)
+    conf.unset("kyuubi.key.alternative")
+    assert(conf.get(config) == 5.0)
+    conf.unset("kyuubi.key.alternative2")
+    assert(conf.get(config) == 3.0)
+  }
 }