From 4901553329da874c8aef6def4d5083f103bda1f1 Mon Sep 17 00:00:00 2001 From: bowenliang Date: Thu, 25 May 2023 10:58:31 +0800 Subject: [PATCH] [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 Signed-off-by: liangbowen --- .../apache/kyuubi/config/ConfigBuilder.scala | 16 ++++++++++ .../org/apache/kyuubi/config/KyuubiConf.scala | 30 +++++++++---------- .../kyuubi/config/ConfigBuilderSuite.scala | 21 +++++++++++++ .../apache/kyuubi/metrics/MetricsConf.scala | 2 +- .../metadata/jdbc/JDBCMetadataStoreConf.scala | 4 +-- 5 files changed, 55 insertions(+), 18 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 62f060a05..8d7501552 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 @@ -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 => diff --git a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala index 9ae1898c5..36fd27285 100644 --- a/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala +++ b/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala @@ -387,8 +387,8 @@ object KyuubiConf { "") .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 { "") .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 { "") .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 { "") .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 { "
  • CUSTOM: to be done.
    • ") .version("1.7.0") .stringConf - .transform(_.toUpperCase(Locale.ROOT)) + .transformToUpperCase .toSequence() .checkValue( _.toSet.subsetOf(Set("JSON", "JDBC", "CUSTOM")), @@ -2855,7 +2855,7 @@ object KyuubiConf { "
  • CUSTOM: to be done.
    • ") .version("1.7.0") .stringConf - .transform(_.toUpperCase(Locale.ROOT)) + .transformToUpperCase .toSequence() .checkValue( _.toSet.subsetOf(Set("JSON", "JDBC", "CUSTOM")), diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigBuilderSuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigBuilderSuite.scala index 4a9ade551..2cb683f52 100644 --- a/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigBuilderSuite.scala +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/config/ConfigBuilderSuite.scala @@ -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") { diff --git a/kyuubi-metrics/src/main/scala/org/apache/kyuubi/metrics/MetricsConf.scala b/kyuubi-metrics/src/main/scala/org/apache/kyuubi/metrics/MetricsConf.scala index ad734ced5..cbdf832bd 100644 --- a/kyuubi-metrics/src/main/scala/org/apache/kyuubi/metrics/MetricsConf.scala +++ b/kyuubi-metrics/src/main/scala/org/apache/kyuubi/metrics/MetricsConf.scala @@ -43,7 +43,7 @@ object MetricsConf { "") .version("1.2.0") .stringConf - .transform(_.toUpperCase()) + .transformToUpperCase .toSequence() .checkValue( _.forall(ReporterType.values.map(_.toString).contains), diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStoreConf.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStoreConf.scala index de30b6e66..0d46fa7fc 100644 --- a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStoreConf.scala +++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/metadata/jdbc/JDBCMetadataStoreConf.scala @@ -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] =