From 11d1de2a0efaaee9eae9d7ab8ed68abf0f26d71e Mon Sep 17 00:00:00 2001 From: timothy65535 Date: Wed, 18 Aug 2021 10:24:59 +0800 Subject: [PATCH] [KYUUBI #942] Support for adding internal attributes to ConfigBuilder ### _Why are the changes needed?_ ### Description `ConfigBuilder` currently only supports public configuration by default. In practice, there are also some configurations, which are internal configurations and do not need to be reserved for users. refer to https://github.com/apache/incubator-kyuubi/pull/935#issuecomment-900117832 ### Modification - Add `internal` to ConfigBuilder - Update `AllKyuubiConfiguration` ### _How was this patch tested?_ - [ ] 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.readthedocs.io/en/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #949 from timothy65535/ky-942. Closes #942 5d4723b3 [timothy65535] update config b41461bb [timothy65535] [KYUUBI #942] Support for adding internal attributes to ConfigBuilder Authored-by: timothy65535 Signed-off-by: Kent Yao --- .../apache/kyuubi/config/ConfigBuilder.scala | 16 ++++++++++++---- .../apache/kyuubi/config/ConfigEntry.scala | 19 ++++++++++++++++--- .../kyuubi/config/ConfigEntrySuite.scala | 18 +++++++++++++----- .../config/AllKyuubiConfiguration.scala | 1 + 4 files changed, 42 insertions(+), 12 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 a0a497a12..70490727e 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 @@ -27,6 +27,12 @@ private[kyuubi] case class ConfigBuilder(key: String) { private[config] var _version = "" private[config] var _onCreate: Option[ConfigEntry[_] => Unit] = None private[config] var _type = "" + private[config] var _internal = false + + def internal: ConfigBuilder = { + _internal = true + this + } def doc(s: String): ConfigBuilder = { _doc = s @@ -107,7 +113,7 @@ private[kyuubi] case class ConfigBuilder(key: String) { def fallbackConf[T](fallback: ConfigEntry[T]): ConfigEntry[T] = { val entry = - new ConfigEntryFallback[T](key, _doc, _version, fallback) + new ConfigEntryFallback[T](key, _doc, _version, _internal, fallback) _onCreate.foreach(_(entry)) entry } @@ -152,7 +158,7 @@ private[kyuubi] case class TypedConfigBuilder[T]( def createOptional: OptionalConfigEntry[T] = { val entry = new OptionalConfigEntry( - parent.key, fromStr, toStr, parent._doc, parent._version, parent._type) + parent.key, fromStr, toStr, parent._doc, parent._version, parent._type, parent._internal) parent._onCreate.foreach(_(entry)) entry } @@ -162,14 +168,16 @@ private[kyuubi] case class TypedConfigBuilder[T]( case _ => val d = fromStr(toStr(default)) val entry = new ConfigEntryWithDefault( - parent.key, d, fromStr, toStr, parent._doc, parent._version, parent._type) + parent.key, d, fromStr, toStr, parent._doc, + parent._version, parent._type, parent._internal) parent._onCreate.foreach(_(entry)) entry } def createWithDefaultString(default: String): ConfigEntryWithDefaultString[T] = { val entry = new ConfigEntryWithDefaultString( - parent.key, default, fromStr, toStr, parent._doc, parent._version, parent._type) + parent.key, default, fromStr, toStr, parent._doc, + parent._version, parent._type, parent._internal) parent._onCreate.foreach(_(entry)) entry } 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 cbf44a776..0637e5b53 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 @@ -24,6 +24,7 @@ trait ConfigEntry[T] { def doc: String def version: String def typ: String + def internal: Boolean def defaultValStr: String def defaultVal: Option[T] @@ -47,7 +48,8 @@ class OptionalConfigEntry[T]( rawStrConverter: T => String, _doc: String, _version: String, - _type: String) extends ConfigEntry[Option[T]] { + _type: String, + _internal: Boolean) extends ConfigEntry[Option[T]] { override def valueConverter: String => Option[T] = { s => Option(rawValueConverter(s)) } @@ -73,6 +75,8 @@ class OptionalConfigEntry[T]( override def version: String = _version override def typ: String = _type + + override def internal: Boolean = _internal } class ConfigEntryWithDefault[T]( @@ -82,7 +86,8 @@ class ConfigEntryWithDefault[T]( _strConverter: T => String, _doc: String, _version: String, - _type: String) extends ConfigEntry[T] { + _type: String, + _internal: Boolean) extends ConfigEntry[T] { override def defaultValStr: String = strConverter(_defaultVal) override def defaultVal: Option[T] = Option(_defaultVal) @@ -102,6 +107,8 @@ class ConfigEntryWithDefault[T]( override def version: String = _version override def typ: String = _type + + override def internal: Boolean = _internal } class ConfigEntryWithDefaultString[T]( @@ -111,7 +118,8 @@ class ConfigEntryWithDefaultString[T]( _strConverter: T => String, _doc: String, _version: String, - _type: String) extends ConfigEntry[T] { + _type: String, + _internal: Boolean) extends ConfigEntry[T] { override def defaultValStr: String = _defaultVal override def defaultVal: Option[T] = Some(valueConverter(_defaultVal)) @@ -132,12 +140,15 @@ class ConfigEntryWithDefaultString[T]( override def version: String = _version override def typ: String = _type + + override def internal: Boolean = _internal } class ConfigEntryFallback[T]( _key: String, _doc: String, _version: String, + _internal: Boolean, fallback: ConfigEntry[T]) extends ConfigEntry[T] { override def defaultValStr: String = fallback.defaultValStr @@ -158,6 +169,8 @@ class ConfigEntryFallback[T]( override def version: String = _version override def typ: String = fallback.typ + + override def internal: Boolean = _internal } object ConfigEntry { 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 2d16d5c4c..e34c78341 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 @@ -29,7 +29,8 @@ class ConfigEntrySuite extends KyuubiFunSuite { v => (v - 1).toString, doc, "", - "int") + "int", + false) assert(e1.key === "kyuubi.int.spark") assert(e1.valueConverter("2") === Some(3)) @@ -39,6 +40,7 @@ class ConfigEntrySuite extends KyuubiFunSuite { assert(e1.doc === doc) assert(e1.version === "") assert(e1.typ === "int") + assert(e1.internal === false) assert(e1.toString === s"ConfigEntry(key=kyuubi.int.spark, defaultValue=," + s" doc=$doc, version=, type=int)") @@ -50,7 +52,8 @@ class ConfigEntrySuite extends KyuubiFunSuite { v => (v - 1).toString, "this is dummy documentation", "", - "int")) + "int", + false)) assert(e.getMessage === "requirement failed: Config entry kyuubi.int.spark already registered!") conf.set(e1.key, "2") @@ -65,7 +68,8 @@ class ConfigEntrySuite extends KyuubiFunSuite { v => (v - 1).toString, "doc", "0.11.1", - "long") + "long", + false) assert(e1.key === "kyuubi.long.spark") assert(e1.valueConverter("2") === 3) @@ -75,6 +79,7 @@ class ConfigEntrySuite extends KyuubiFunSuite { assert(e1.doc === "doc") assert(e1.version === "0.11.1") assert(e1.typ === "long") + assert(e1.internal === false) assert(e1.toString === s"ConfigEntry(key=kyuubi.long.spark, defaultValue=1," + s" doc=doc, version=0.11.1, type=long)") @@ -92,7 +97,8 @@ class ConfigEntrySuite extends KyuubiFunSuite { v => v.toString, "doc", "", - "double") + "double", + false) assert(e1.key === "kyuubi.double.spark") assert(e1.valueConverter("2") === 2.0) @@ -102,6 +108,7 @@ class ConfigEntrySuite extends KyuubiFunSuite { assert(e1.doc === "doc") assert(e1.version === "") assert(e1.typ === "double") + assert(e1.internal === false) assert(e1.toString === s"ConfigEntry(key=kyuubi.double.spark, defaultValue=3.0," + s" doc=doc, version=, type=double)") @@ -116,7 +123,7 @@ class ConfigEntrySuite extends KyuubiFunSuite { .version("1.1.1") .stringConf.createWithDefault("origin") val fallback = - new ConfigEntryFallback[String]("kyuubi.fallback.spark", "fallback", "1.2.0", origin) + new ConfigEntryFallback[String]("kyuubi.fallback.spark", "fallback", "1.2.0", false, origin) assert(fallback.key === "kyuubi.fallback.spark") assert(fallback.valueConverter("2") === "2") @@ -126,6 +133,7 @@ class ConfigEntrySuite extends KyuubiFunSuite { assert(fallback.doc === "fallback") assert(fallback.version === "1.2.0") assert(fallback.typ === "string") + assert(fallback.internal === false) assert(fallback.toString === s"ConfigEntry(key=kyuubi.fallback.spark, defaultValue=origin," + s" doc=fallback, version=1.2.0, type=string)") diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/config/AllKyuubiConfiguration.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/config/AllKyuubiConfiguration.scala index 7f6e043ca..e5f9ca74e 100644 --- a/kyuubi-server/src/test/scala/org/apache/kyuubi/config/AllKyuubiConfiguration.scala +++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/config/AllKyuubiConfiguration.scala @@ -119,6 +119,7 @@ class AllKyuubiConfiguration extends KyuubiFunSuite { KyuubiConf.kyuubiConfEntries.values().asScala .toSeq + .filterNot(_.internal) .groupBy(_.key.split("\\.")(1)) .toSeq.sortBy(_._1).foreach { case (category, entries) =>