jsancio commented on a change in pull request #11711:
URL: https://github.com/apache/kafka/pull/11711#discussion_r791893829
##########
File path: core/src/main/scala/kafka/server/ConfigAdminManager.scala
##########
@@ -515,4 +512,16 @@ object ConfigAdminManager {
}
}
}
+
+ def getConfigPropertyAsList(
+ configProps: Properties,
+ configKeys: Map[String, ConfigKey],
+ configPropName: String
+ ): List[String] = {
+ Option(configProps.getProperty(configPropName))
+
.orElse(Option(ConfigDef.convertToString(configKeys(configPropName).defaultValue,
ConfigDef.Type.LIST)))
+ .filter(_.trim.nonEmpty)
+ .map(_.split(",").toList)
Review comment:
Do you think that we should trim and filter after splitting based on
commas? For example should `"first , second, third"` be equivalent to
`"first,second,third"`?
##########
File path: core/src/main/scala/kafka/server/ConfigAdminManager.scala
##########
@@ -495,10 +495,7 @@ object ConfigAdminManager {
case OpType.APPEND => {
if (!listType(alterConfigOp.configEntry.name, configKeys))
throw new InvalidRequestException(s"Config value append is not
allowed for config key: ${alterConfigOp.configEntry.name}")
- val oldValueList =
Option(configProps.getProperty(alterConfigOp.configEntry.name))
-
.orElse(Option(ConfigDef.convertToString(configKeys(configPropName).defaultValue,
ConfigDef.Type.LIST)))
- .getOrElse("")
- .split(",").toList
+ val oldValueList = getConfigPropertyAsList(configProps, configKeys,
configPropName)
val newValueList = oldValueList :::
alterConfigOp.configEntry.value.split(",").toList
Review comment:
Do you think that we should also `trim` the value that is getting
appended? If I understand this code correctly, Kafka will store the white
space but a future `APPEND` will trim them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
