From 68ac8a19f3a8beafc628e204da4ac03a759b143b Mon Sep 17 00:00:00 2001 From: Min Zhao Date: Tue, 26 Apr 2022 09:40:42 +0800 Subject: [PATCH] [KYUUBI #2453] [Improvement] checkValue of TypedConfigBuilder shall also print the config name ### _Why are the changes needed?_ ### _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 - [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request Closes #2465 from zhaomin1423/2453. Closes #2453 8a624991 [Min Zhao] fix tset 54ee09df [Min Zhao] fix suite 3d434a78 [Min Zhao] [KYUUBI #2453] [Improvement] checkValue of TypedConfigBuilder shall also print the config name f7c7bf27 [Min Zhao] [KYUUBI #2453] [Improvement] checkValue of TypedConfigBuilder shall also print the config name Authored-by: Min Zhao Signed-off-by: ulysses-you --- .../org/apache/kyuubi/config/ConfigBuilder.scala | 4 +++- .../apache/kyuubi/config/ConfigBuilderSuite.scala | 12 ++++++++++++ .../KyuubiAuthenticationFactorySuite.scala | 2 +- .../org/apache/kyuubi/server/KyuubiServerSuite.scala | 2 +- 4 files changed, 17 insertions(+), 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 index 9776273ca..8863aabc3 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 @@ -136,7 +136,9 @@ private[kyuubi] case class 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 => - if (!validator(v)) throw new IllegalArgumentException(errMsg) + if (!validator(v)) { + throw new IllegalArgumentException(s"'$v' in ${parent.key} is invalid. $errMsg") + } v } } 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 df169643a..73d5fbde7 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 @@ -81,4 +81,16 @@ class ConfigBuilderSuite extends KyuubiFunSuite { val e = intercept[IllegalArgumentException](kyuubiConf.get(timeConf)) assert(e.getMessage startsWith "The formats accepted are 1) based on the ISO-8601") } + + test("invalid config") { + val intConf = ConfigBuilder("kyuubi.invalid.config") + .intConf + .checkValue(t => t > 0, "must be positive integer") + .createWithDefault(3) + assert(intConf.key === "kyuubi.invalid.config") + assert(intConf.defaultVal.get === 3) + val kyuubiConf = KyuubiConf().set(intConf.key, "-1") + val e = intercept[IllegalArgumentException](kyuubiConf.get(intConf)) + assert(e.getMessage equals "'-1' in kyuubi.invalid.config is invalid. must be positive integer") + } } diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/KyuubiAuthenticationFactorySuite.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/KyuubiAuthenticationFactorySuite.scala index 753374e31..bfcd3011e 100644 --- a/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/KyuubiAuthenticationFactorySuite.scala +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/service/authentication/KyuubiAuthenticationFactorySuite.scala @@ -58,7 +58,7 @@ class KyuubiAuthenticationFactorySuite extends KyuubiFunSuite { test("AuthType Other") { val conf = KyuubiConf().set(KyuubiConf.AUTHENTICATION_METHOD, Seq("INVALID")) val e = intercept[IllegalArgumentException](new KyuubiAuthenticationFactory(conf)) - assert(e.getMessage === "the authentication type should be one or more of" + + assert(e.getMessage contains "the authentication type should be one or more of" + " NOSASL,NONE,LDAP,KERBEROS,CUSTOM") } diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/KyuubiServerSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/KyuubiServerSuite.scala index fba26709d..d25d4fa05 100644 --- a/kyuubi-server/src/test/scala/org/apache/kyuubi/server/KyuubiServerSuite.scala +++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/server/KyuubiServerSuite.scala @@ -92,7 +92,7 @@ class KyuubiServerSuite extends KyuubiFunSuite { test("invalid port") { val conf = KyuubiConf().set(KyuubiConf.FRONTEND_THRIFT_BINARY_BIND_PORT, 100) val e = intercept[IllegalArgumentException](new KyuubiServer().initialize(conf)) - assert(e.getMessage === "Invalid Port number") + assert(e.getMessage contains "Invalid Port number") } test("invalid zookeeper quorum") {