[KYUUBI #4812] [MINOR] Generalize case transformation method for string type config entry

### _Why are the changes needed?_

- unify config value capitalization for String and Seq[String] configs
- Generalize `transformToUpperCase` and `transformToLowerCase` for config entry
- simplify transformation for configs of `kyuubi.authentication`  and `kyuubi.frontend.protocols`

### _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/master/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #4812 from bowenliang123/conf-upper.

Closes #4812

747c2955c [bowenliang] upper and lower case for config values

Authored-by: bowenliang <bowenliang@apache.org>
Signed-off-by: liangbowen <liangbowen@gf.com.cn>
This commit is contained in:
bowenliang 2023-05-25 10:58:31 +08:00 committed by liangbowen
parent 320178bb68
commit 4901553329
5 changed files with 55 additions and 18 deletions

View File

@ -18,6 +18,7 @@
package org.apache.kyuubi.config
import java.time.Duration
import java.util.Locale
import java.util.regex.PatternSyntaxException
import scala.util.{Failure, Success, Try}
@ -166,6 +167,21 @@ private[kyuubi] case class TypedConfigBuilder[T](
def transform(fn: T => T): TypedConfigBuilder[T] = this.copy(fromStr = s => fn(fromStr(s)))
def transformToUpperCase: TypedConfigBuilder[T] = {
transformString(_.toUpperCase(Locale.ROOT))
}
def transformToLowerCase: TypedConfigBuilder[T] = {
transformString(_.toLowerCase(Locale.ROOT))
}
private def transformString(fn: String => String): TypedConfigBuilder[T] = {
require(parent._type == "string")
this.asInstanceOf[TypedConfigBuilder[String]]
.transform(fn)
.asInstanceOf[TypedConfigBuilder[T]]
}
/** Checks if the user-provided value for the config matches the validator. */
def checkValue(validator: T => Boolean, errMsg: String): TypedConfigBuilder[T] = {
transform { v =>

View File

@ -387,8 +387,8 @@ object KyuubiConf {
"</ul>")
.version("1.4.0")
.stringConf
.transformToUpperCase
.toSequence()
.transform(_.map(_.toUpperCase(Locale.ROOT)))
.checkValue(
_.forall(FrontendProtocols.values.map(_.toString).contains),
s"the frontend protocol should be one or more of ${FrontendProtocols.values.mkString(",")}")
@ -764,8 +764,8 @@ object KyuubiConf {
.version("1.0.0")
.serverOnly
.stringConf
.transformToUpperCase
.toSequence()
.transform(_.map(_.toUpperCase(Locale.ROOT)))
.checkValue(
_.forall(AuthTypes.values.map(_.toString).contains),
s"the authentication type should be one or more of ${AuthTypes.values.mkString(",")}")
@ -1001,7 +1001,7 @@ object KyuubiConf {
.serverOnly
.stringConf
.checkValues(SaslQOP.values.map(_.toString))
.transform(_.toLowerCase(Locale.ROOT))
.transformToLowerCase
.createWithDefault(SaslQOP.AUTH.toString)
val FRONTEND_REST_BIND_HOST: ConfigEntry[Option[String]] =
@ -1733,7 +1733,7 @@ object KyuubiConf {
.version("1.7.0")
.stringConf
.checkValues(Set("arrow", "thrift"))
.transform(_.toLowerCase(Locale.ROOT))
.transformToLowerCase
.createWithDefault("thrift")
val ARROW_BASED_ROWSET_TIMESTAMP_AS_STRING: ConfigEntry[Boolean] =
@ -1758,7 +1758,7 @@ object KyuubiConf {
.doc(s"(deprecated) - Using kyuubi.engine.share.level instead")
.version("1.0.0")
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.transformToUpperCase
.checkValues(ShareLevel.values.map(_.toString))
.createWithDefault(ShareLevel.USER.toString)
@ -1773,7 +1773,7 @@ object KyuubiConf {
.doc("(deprecated) - Using kyuubi.engine.share.level.subdomain instead")
.version("1.2.0")
.stringConf
.transform(_.toLowerCase(Locale.ROOT))
.transformToLowerCase
.checkValue(validZookeeperSubPath.matcher(_).matches(), "must be valid zookeeper sub path.")
.createOptional
@ -1839,7 +1839,7 @@ object KyuubiConf {
"</ul>")
.version("1.4.0")
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.transformToUpperCase
.checkValues(EngineType.values.map(_.toString))
.createWithDefault(EngineType.SPARK_SQL.toString)
@ -1886,7 +1886,7 @@ object KyuubiConf {
"</ul>")
.version("1.7.0")
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.transformToUpperCase
.checkValues(Set("RANDOM", "POLLING"))
.createWithDefault("RANDOM")
@ -2047,7 +2047,7 @@ object KyuubiConf {
.version("1.4.0")
.serverOnly
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.transformToUpperCase
.toSequence()
.checkValue(
_.toSet.subsetOf(Set("JSON", "JDBC", "CUSTOM", "KAFKA")),
@ -2071,7 +2071,7 @@ object KyuubiConf {
" which has a zero-arg constructor.")
.version("1.3.0")
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.transformToUpperCase
.toSequence()
.checkValue(
_.toSet.subsetOf(Set("SPARK", "JSON", "JDBC", "CUSTOM")),
@ -2204,7 +2204,7 @@ object KyuubiConf {
"'physical', and 'execution', other engines do not support planOnly currently.")
.version("1.4.0")
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.transformToUpperCase
.checkValue(
mode =>
Set(
@ -2227,7 +2227,7 @@ object KyuubiConf {
"of the Spark engine")
.version("1.7.0")
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.transformToUpperCase
.checkValue(
mode => Set("PLAIN", "JSON").contains(mode),
"Invalid value for 'kyuubi.operation.plan.only.output.style'. Valid values are " +
@ -2278,7 +2278,7 @@ object KyuubiConf {
"</ul>")
.version("1.5.0")
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.transformToUpperCase
.checkValues(OperationLanguages.values.map(_.toString))
.createWithDefault(OperationLanguages.SQL.toString)
@ -2838,7 +2838,7 @@ object KyuubiConf {
" <li>CUSTOM: to be done.</li></ul>")
.version("1.7.0")
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.transformToUpperCase
.toSequence()
.checkValue(
_.toSet.subsetOf(Set("JSON", "JDBC", "CUSTOM")),
@ -2855,7 +2855,7 @@ object KyuubiConf {
" <li>CUSTOM: to be done.</li></ul>")
.version("1.7.0")
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.transformToUpperCase
.toSequence()
.checkValue(
_.toSet.subsetOf(Set("JSON", "JDBC", "CUSTOM")),

View File

@ -72,6 +72,27 @@ class ConfigBuilderSuite extends KyuubiFunSuite {
KyuubiConf.register(sequenceConf)
val kyuubiConf = KyuubiConf().set(sequenceConf.key, "kyuubi,kent")
assert(kyuubiConf.get(sequenceConf) === Seq("kyuubi", "kent"))
val stringConfUpper = ConfigBuilder("kyuubi.string.conf.upper")
.stringConf
.transformToUpperCase
.createWithDefault("Kent, Yao")
assert(stringConfUpper.key === "kyuubi.string.conf.upper")
assert(stringConfUpper.defaultVal.get === "KENT, YAO")
val stringConfUpperSeq = ConfigBuilder("kyuubi.string.conf.upper.seq")
.stringConf
.transformToUpperCase
.toSequence()
.createWithDefault(Seq("hehe"))
assert(stringConfUpperSeq.defaultVal.get === Seq("HEHE"))
val stringConfLower = ConfigBuilder("kyuubi.string.conf.lower")
.stringConf
.transformToLowerCase
.createWithDefault("Kent, Yao")
assert(stringConfLower.key === "kyuubi.string.conf.lower")
assert(stringConfLower.defaultVal.get === "kent, yao")
}
test("time config") {

View File

@ -43,7 +43,7 @@ object MetricsConf {
"</ul>")
.version("1.2.0")
.stringConf
.transform(_.toUpperCase())
.transformToUpperCase
.toSequence()
.checkValue(
_.forall(ReporterType.values.map(_.toString).contains),

View File

@ -17,7 +17,7 @@
package org.apache.kyuubi.server.metadata.jdbc
import java.util.{Locale, Properties}
import java.util.Properties
import org.apache.kyuubi.config.{ConfigEntry, KyuubiConf, OptionalConfigEntry}
import org.apache.kyuubi.config.KyuubiConf.buildConf
@ -46,7 +46,7 @@ object JDBCMetadataStoreConf {
.version("1.6.0")
.serverOnly
.stringConf
.transform(_.toUpperCase(Locale.ROOT))
.transformToUpperCase
.createWithDefault("DERBY")
val METADATA_STORE_JDBC_DATABASE_SCHEMA_INIT: ConfigEntry[Boolean] =