Skip to content

[Improvement] Use Set collection for order-insensitive configs #5185

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[Improvement] Use Set collection for order-insensitive configs #5185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions docs/configuration/settings.md
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions ...ql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/PlanOnlyStatement.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ class PlanOnlyStatement(
extends SparkOperation(session) {

private val operationLog: OperationLog = OperationLog.createOperationLog(session, getHandle)
private val planExcludes: Seq[String] = {
spark.conf.getOption(OPERATION_PLAN_ONLY_EXCLUDES.key).map(_.split(",").map(_.trim).toSeq)
private val planExcludes: Set[String] = {
spark.conf.getOption(OPERATION_PLAN_ONLY_EXCLUDES.key).map(_.split(",").map(_.trim).toSet)
.getOrElse(session.sessionManager.getConf.get(OPERATION_PLAN_ONLY_EXCLUDES))
}

Expand Down
6 changes: 3 additions & 3 deletions ...uubi-spark-sql-engine/src/main/scala/org/apache/spark/kyuubi/SparkSQLEngineListener.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,9 @@ import org.apache.kyuubi.service.{Serverable, ServiceState}
class SparkSQLEngineListener(server: Serverable) extends SparkListener with Logging {

// the conf of server is null before initialized, use lazy val here
private lazy val deregisterExceptions: Seq[String] =
private lazy val deregisterExceptions: Set[String] =
server.getConf.get(ENGINE_DEREGISTER_EXCEPTION_CLASSES)
private lazy val deregisterMessages: Seq[String] =
private lazy val deregisterMessages: Set[String] =
server.getConf.get(ENGINE_DEREGISTER_EXCEPTION_MESSAGES)
private lazy val deregisterExceptionTTL: Long =
server.getConf.get(ENGINE_DEREGISTER_EXCEPTION_TTL)
Expand Down Expand Up @@ -74,7 +74,7 @@ class SparkSQLEngineListener(server: Serverable) extends SparkListener with Logg
case JobFailed(e) if e != null =>
val cause = findCause(e)
var deregisterInfo: Option[String] = None
if (deregisterExceptions.exists(_.equals(cause.getClass.getCanonicalName))) {
if (deregisterExceptions.contains(cause.getClass.getCanonicalName)) {
deregisterInfo = Some("Job failed exception class is in the set of " +
s"${ENGINE_DEREGISTER_EXCEPTION_CLASSES.key}, deregistering the engine.")
} else if (deregisterMessages.exists(stringifyException(cause).contains)) {
Expand Down
7 changes: 6 additions & 1 deletion kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigBuilder.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,12 @@ private[kyuubi] case class TypedConfigBuilder[T](
/** Turns the config entry into a sequence of values of the underlying type. */
def toSequence(sp: String = ","): TypedConfigBuilder[Seq[T]] = {
parent._type = "seq"
TypedConfigBuilder(parent, strToSeq(_, fromStr, sp), seqToStr(_, toStr))
TypedConfigBuilder(parent, strToSeq(_, fromStr, sp), iterableToStr(_, toStr))
}

def toSet(sp: String = ",", skipBlank: Boolean = true): TypedConfigBuilder[Set[T]] = {
parent._type = "set"
TypedConfigBuilder(parent, strToSet(_, fromStr, sp, skipBlank), iterableToStr(_, toStr))
}

def createOptional: OptionalConfigEntry[T] = {
Expand Down
10 changes: 8 additions & 2 deletions kyuubi-common/src/main/scala/org/apache/kyuubi/config/ConfigHelpers.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package org.apache.kyuubi.config

import org.apache.commons.lang3.StringUtils

import org.apache.kyuubi.Utils

object ConfigHelpers {
Expand All @@ -25,7 +27,11 @@ object ConfigHelpers {
Utils.strToSeq(str, sp).map(converter)
}

def seqToStr[T](v: Seq[T], stringConverter: T => String): String = {
v.map(stringConverter).mkString(",")
def strToSet[T](str: String, converter: String => T, sp: String, skipBlank: Boolean): Set[T] = {
Utils.strToSeq(str, sp).filter(!skipBlank || StringUtils.isNotBlank(_)).map(converter).toSet
}

def iterableToStr[T](v: Iterable[T], stringConverter: T => String, sp: String = ","): String = {
v.map(stringConverter).mkString(sp)
}
}
90 changes: 45 additions & 45 deletions kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -483,13 +483,13 @@ object KyuubiConf {
.stringConf
.createOptional

val FRONTEND_THRIFT_BINARY_SSL_DISALLOWED_PROTOCOLS: ConfigEntry[Seq[String]] =
val FRONTEND_THRIFT_BINARY_SSL_DISALLOWED_PROTOCOLS: ConfigEntry[Set[String]] =
buildConf("kyuubi.frontend.thrift.binary.ssl.disallowed.protocols")
.doc("SSL versions to disallow for Kyuubi thrift binary frontend.")
.version("1.7.0")
.stringConf
.toSequence()
.createWithDefault(Seq("SSLv2", "SSLv3"))
.toSet()
.createWithDefault(Set("SSLv2", "SSLv3"))

val FRONTEND_THRIFT_BINARY_SSL_INCLUDE_CIPHER_SUITES: ConfigEntry[Seq[String]] =
buildConf("kyuubi.frontend.thrift.binary.ssl.include.ciphersuites")
Expand Down Expand Up @@ -765,7 +765,7 @@ object KyuubiConf {
.stringConf
.createWithDefault("X-Real-IP")

val AUTHENTICATION_METHOD: ConfigEntry[Seq[String]] = buildConf("kyuubi.authentication")
val AUTHENTICATION_METHOD: ConfigEntry[Set[String]] = buildConf("kyuubi.authentication")
.doc("A comma-separated list of client authentication types." +
"<ul>" +
" <li>NOSASL: raw transport.</li>" +
Expand Down Expand Up @@ -801,11 +801,11 @@ object KyuubiConf {
.serverOnly
.stringConf
.transformToUpperCase
.toSequence()
.toSet()
.checkValue(
_.forall(AuthTypes.values.map(_.toString).contains),
s"the authentication type should be one or more of ${AuthTypes.values.mkString(",")}")
.createWithDefault(Seq(AuthTypes.NONE.toString))
.createWithDefault(Set(AuthTypes.NONE.toString))

val AUTHENTICATION_CUSTOM_CLASS: OptionalConfigEntry[String] =
buildConf("kyuubi.authentication.custom.class")
Expand Down Expand Up @@ -861,25 +861,25 @@ object KyuubiConf {
.stringConf
.createOptional

val AUTHENTICATION_LDAP_GROUP_FILTER: ConfigEntry[Seq[String]] =
val AUTHENTICATION_LDAP_GROUP_FILTER: ConfigEntry[Set[String]] =
buildConf("kyuubi.authentication.ldap.groupFilter")
.doc("COMMA-separated list of LDAP Group names (short name not full DNs). " +
"For example: HiveAdmins,HadoopAdmins,Administrators")
.version("1.7.0")
.serverOnly
.stringConf
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val AUTHENTICATION_LDAP_USER_FILTER: ConfigEntry[Seq[String]] =
val AUTHENTICATION_LDAP_USER_FILTER: ConfigEntry[Set[String]] =
buildConf("kyuubi.authentication.ldap.userFilter")
.doc("COMMA-separated list of LDAP usernames (just short names, not full DNs). " +
"For example: hiveuser,impalauser,hiveadmin,hadoopadmin")
.version("1.7.0")
.serverOnly
.stringConf
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val AUTHENTICATION_LDAP_GUID_KEY: ConfigEntry[String] =
buildConf("kyuubi.authentication.ldap.guidKey")
Expand Down Expand Up @@ -1142,14 +1142,14 @@ object KyuubiConf {
.stringConf
.createOptional

val KUBERNETES_CONTEXT_ALLOW_LIST: ConfigEntry[Seq[String]] =
val KUBERNETES_CONTEXT_ALLOW_LIST: ConfigEntry[Set[String]] =
buildConf("kyuubi.kubernetes.context.allow.list")
.doc("The allowed kubernetes context list, if it is empty," +
" there is no kubernetes context limitation.")
.version("1.8.0")
.stringConf
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val KUBERNETES_NAMESPACE: ConfigEntry[String] =
buildConf("kyuubi.kubernetes.namespace")
Expand All @@ -1158,14 +1158,14 @@ object KyuubiConf {
.stringConf
.createWithDefault("default")

val KUBERNETES_NAMESPACE_ALLOW_LIST: ConfigEntry[Seq[String]] =
val KUBERNETES_NAMESPACE_ALLOW_LIST: ConfigEntry[Set[String]] =
buildConf("kyuubi.kubernetes.namespace.allow.list")
.doc("The allowed kubernetes namespace list, if it is empty," +
" there is no kubernetes namespace limitation.")
.version("1.8.0")
.stringConf
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val KUBERNETES_MASTER: OptionalConfigEntry[String] =
buildConf("kyuubi.kubernetes.master.address")
Expand Down Expand Up @@ -1517,7 +1517,7 @@ object KyuubiConf {
.timeConf
.createWithDefault(Duration.ofMinutes(30L).toMillis)

val SESSION_CONF_IGNORE_LIST: ConfigEntry[Seq[String]] =
val SESSION_CONF_IGNORE_LIST: ConfigEntry[Set[String]] =
buildConf("kyuubi.session.conf.ignore.list")
.doc("A comma-separated list of ignored keys. If the client connection contains any of" +
" them, the key and the corresponding value will be removed silently during engine" +
Expand All @@ -1527,10 +1527,10 @@ object KyuubiConf {
" configurations via SET syntax.")
.version("1.2.0")
.stringConf
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val SESSION_CONF_RESTRICT_LIST: ConfigEntry[Seq[String]] =
val SESSION_CONF_RESTRICT_LIST: ConfigEntry[Set[String]] =
buildConf("kyuubi.session.conf.restrict.list")
.doc("A comma-separated list of restricted keys. If the client connection contains any of" +
" them, the connection will be rejected explicitly during engine bootstrap and connection" +
Expand All @@ -1540,8 +1540,8 @@ object KyuubiConf {
" configurations via SET syntax.")
.version("1.2.0")
.stringConf
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val SESSION_USER_SIGN_ENABLED: ConfigEntry[Boolean] =
buildConf("kyuubi.session.user.sign.enabled")
Expand Down Expand Up @@ -1589,7 +1589,7 @@ object KyuubiConf {
.booleanConf
.createWithDefault(true)

val SESSION_LOCAL_DIR_ALLOW_LIST: ConfigEntry[Seq[String]] =
val SESSION_LOCAL_DIR_ALLOW_LIST: ConfigEntry[Set[String]] =
buildConf("kyuubi.session.local.dir.allow.list")
.doc("The local dir list that are allowed to access by the kyuubi session application. " +
" End-users might set some parameters such as `spark.files` and it will " +
Expand All @@ -1602,8 +1602,8 @@ object KyuubiConf {
.stringConf
.checkValue(dir => dir.startsWith(File.separator), "the dir should be absolute path")
.transform(dir => dir.stripSuffix(File.separator) + File.separator)
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val BATCH_APPLICATION_CHECK_INTERVAL: ConfigEntry[Long] =
buildConf("kyuubi.batch.application.check.interval")
Expand All @@ -1619,7 +1619,7 @@ object KyuubiConf {
.timeConf
.createWithDefault(Duration.ofMinutes(3).toMillis)

val BATCH_CONF_IGNORE_LIST: ConfigEntry[Seq[String]] =
val BATCH_CONF_IGNORE_LIST: ConfigEntry[Set[String]] =
buildConf("kyuubi.batch.conf.ignore.list")
.doc("A comma-separated list of ignored keys for batch conf. If the batch conf contains" +
" any of them, the key and the corresponding value will be removed silently during batch" +
Expand All @@ -1631,8 +1631,8 @@ object KyuubiConf {
" for the Spark batch job with key `kyuubi.batchConf.spark.spark.master`.")
.version("1.6.0")
.stringConf
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val BATCH_INTERNAL_REST_CLIENT_SOCKET_TIMEOUT: ConfigEntry[Long] =
buildConf("kyuubi.batch.internal.rest.client.socket.timeout")
Expand Down Expand Up @@ -2076,24 +2076,24 @@ object KyuubiConf {
.toSequence(";")
.createWithDefault(Nil)

val ENGINE_DEREGISTER_EXCEPTION_CLASSES: ConfigEntry[Seq[String]] =
val ENGINE_DEREGISTER_EXCEPTION_CLASSES: ConfigEntry[Set[String]] =
buildConf("kyuubi.engine.deregister.exception.classes")
.doc("A comma-separated list of exception classes. If there is any exception thrown," +
" whose class matches the specified classes, the engine would deregister itself.")
.version("1.2.0")
.stringConf
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val ENGINE_DEREGISTER_EXCEPTION_MESSAGES: ConfigEntry[Seq[String]] =
val ENGINE_DEREGISTER_EXCEPTION_MESSAGES: ConfigEntry[Set[String]] =
buildConf("kyuubi.engine.deregister.exception.messages")
.doc("A comma-separated list of exception messages. If there is any exception thrown," +
" whose message or stacktrace matches the specified message list, the engine would" +
" deregister itself.")
.version("1.2.0")
.stringConf
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val ENGINE_DEREGISTER_JOB_MAX_FAILURES: ConfigEntry[Int] =
buildConf("kyuubi.engine.deregister.job.max.failures")
Expand Down Expand Up @@ -2400,7 +2400,7 @@ object KyuubiConf {
"'plain', 'json'.")
.createWithDefault(PlainStyle.name)

val OPERATION_PLAN_ONLY_EXCLUDES: ConfigEntry[Seq[String]] =
val OPERATION_PLAN_ONLY_EXCLUDES: ConfigEntry[Set[String]] =
buildConf("kyuubi.operation.plan.only.excludes")
.doc("Comma-separated list of query plan names, in the form of simple class names, i.e, " +
"for `SET abc=xyz`, the value will be `SetCommand`. For those auxiliary plans, such as " +
Expand All @@ -2410,8 +2410,8 @@ object KyuubiConf {
s"See also ${OPERATION_PLAN_ONLY_MODE.key}.")
.version("1.5.0")
.stringConf
.toSequence()
.createWithDefault(Seq(
.toSet()
.createWithDefault(Set(
"ResetCommand",
"SetCommand",
"SetNamespaceCommand",
Expand Down Expand Up @@ -2614,14 +2614,14 @@ object KyuubiConf {
.intConf
.createOptional

val SERVER_LIMIT_CONNECTIONS_USER_UNLIMITED_LIST: ConfigEntry[Seq[String]] =
val SERVER_LIMIT_CONNECTIONS_USER_UNLIMITED_LIST: ConfigEntry[Set[String]] =
buildConf("kyuubi.server.limit.connections.user.unlimited.list")
.doc("The maximum connections of the user in the white list will not be limited.")
.version("1.7.0")
.serverOnly
.stringConf
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val SERVER_LIMIT_BATCH_CONNECTIONS_PER_USER: OptionalConfigEntry[Int] =
buildConf("kyuubi.server.limit.batch.connections.per.user")
Expand Down Expand Up @@ -2683,15 +2683,15 @@ object KyuubiConf {
.timeConf
.createWithDefaultString("PT30M")

val SERVER_ADMINISTRATORS: ConfigEntry[Seq[String]] =
val SERVER_ADMINISTRATORS: ConfigEntry[Set[String]] =
buildConf("kyuubi.server.administrators")
.doc("Comma-separated list of Kyuubi service administrators. " +
"We use this config to grant admin permission to any service accounts.")
.version("1.8.0")
.serverOnly
.stringConf
.toSequence()
.createWithDefault(Nil)
.toSet()
.createWithDefault(Set.empty)

val OPERATION_SPARK_LISTENER_ENABLED: ConfigEntry[Boolean] =
buildConf("kyuubi.operation.spark.listener.enabled")
Expand Down
2 changes: 1 addition & 1 deletion kyuubi-common/src/main/scala/org/apache/kyuubi/service/TBinaryFrontendService.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ abstract class TBinaryFrontendService(name: String)
keyStorePassword: String,
keyStoreType: Option[String],
keyStoreAlgorithm: Option[String],
disallowedSslProtocols: Seq[String],
disallowedSslProtocols: Set[String],
includeCipherSuites: Seq[String]): TServerSocket = {
val params =
if (includeCipherSuites.nonEmpty) {
Expand Down
2 changes: 1 addition & 1 deletion ...src/main/scala/org/apache/kyuubi/service/authentication/KyuubiAuthenticationFactory.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class KyuubiAuthenticationFactory(conf: KyuubiConf, isServer: Boolean = true) ex

private val authTypes = conf.get(AUTHENTICATION_METHOD).map(AuthTypes.withName)
private val none = authTypes.contains(NONE)
private val noSasl = authTypes == Seq(NOSASL)
private val noSasl = authTypes == Set(NOSASL)
private val kerberosEnabled = authTypes.contains(KERBEROS)
private val plainAuthTypeOpt = authTypes.filterNot(_.equals(KERBEROS))
.filterNot(_.equals(NOSASL)).headOption
Expand Down
4 changes: 2 additions & 2 deletions ...mon/src/main/scala/org/apache/kyuubi/service/authentication/ldap/GroupFilterFactory.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ object GroupFilterFactory extends FilterFactory {
}
}

class GroupMembershipKeyFilter(groupFilter: Seq[String]) extends Filter with Logging {
class GroupMembershipKeyFilter(groupFilter: Set[String]) extends Filter with Logging {

@throws[AuthenticationException]
override def apply(ldap: DirSearch, user: String): Unit = {
Expand Down Expand Up @@ -70,7 +70,7 @@ class GroupMembershipKeyFilter(groupFilter: Seq[String]) extends Filter with Log
}
}

class UserMembershipKeyFilter(groupFilter: Seq[String]) extends Filter with Logging {
class UserMembershipKeyFilter(groupFilter: Set[String]) extends Filter with Logging {
@throws[AuthenticationException]
override def apply(ldap: DirSearch, user: String): Unit = {
info(s"Authenticating user '$user' using $classOf[UserMembershipKeyFilter].getSimpleName")
Expand Down
4 changes: 2 additions & 2 deletions ...mmon/src/main/scala/org/apache/kyuubi/service/authentication/ldap/UserFilterFactory.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ object UserFilterFactory extends FilterFactory with Logging {
}
}

class UserFilter(_userFilter: Seq[String]) extends Filter with Logging {
class UserFilter(_userFilter: Set[String]) extends Filter with Logging {

lazy val userFilter: Seq[String] = _userFilter.map(_.toLowerCase)
lazy val userFilter: Set[String] = _userFilter.map(_.toLowerCase)

@throws[AuthenticationException]
override def apply(ldap: DirSearch, user: String): Unit = {
Expand Down
Loading