From 53eccf99eacd398ee8485d1142879979600dd51b Mon Sep 17 00:00:00 2001 From: Fei Wang Date: Mon, 17 Oct 2022 23:54:41 +0800 Subject: [PATCH 1/3] Support alternative keys in ConfigBuilder --- .../org/apache/kyuubi/config/ConfigBuilder.scala | 6 ++++++ .../org/apache/kyuubi/config/ConfigEntry.scala | 15 ++++++++++++++- 2 files changed, 20 insertions(+), 1 deletion(-) 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 index d834de44a42..859dae8685c 100644 --- 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) 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 index 5072031654c..5b5d6706b04 100644 --- 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 From a2300b268b07aed6d089d21e232e7aa86d46fe98 Mon Sep 17 00:00:00 2001 From: Fei Wang Date: Tue, 18 Oct 2022 10:59:49 +0800 Subject: [PATCH 2/3] add ut --- .../apache/kyuubi/config/ConfigBuilder.scala | 5 ++- .../kyuubi/config/ConfigEntrySuite.scala | 31 ++++++++++++++++++- 2 files changed, 34 insertions(+), 2 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 index 859dae8685c..e82e55b23f2 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigBuilder.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigBuilder.scala @@ -123,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 } @@ -183,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, @@ -199,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, @@ -213,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/test/scala/org/apache/kyuubi/config/ConfigEntrySuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigEntrySuite.scala index 670b829e729..cb20c9995a8 100644 --- 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,22 @@ 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"), + "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") + assert(conf.get(config) == 4.0) + } } From e268fef2653c798dc3662761dc10419a052919df Mon Sep 17 00:00:00 2001 From: Fei Wang Date: Tue, 18 Oct 2022 11:00:48 +0800 Subject: [PATCH 3/3] add ut --- .../scala/org/apache/kyuubi/config/ConfigEntrySuite.scala | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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 index cb20c9995a8..d8f004ff9be 100644 --- a/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigEntrySuite.scala +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigEntrySuite.scala @@ -185,7 +185,7 @@ class ConfigEntrySuite extends KyuubiFunSuite { test("support alternative keys in ConfigBuilder") { val config = new ConfigEntryWithDefaultString[Double]( "kyuubi.key", - List("kyuubi.key.alternative"), + List("kyuubi.key.alternative", "kyuubi.key.alternative2"), "3.0", s => java.lang.Double.valueOf(s), v => v.toString, @@ -198,6 +198,11 @@ class ConfigEntrySuite extends KyuubiFunSuite { 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) } }