OmniaGM commented on code in PR #15569:
URL: https://github.com/apache/kafka/pull/15569#discussion_r1541026665
##########
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##########
@@ -49,6 +49,7 @@ import
org.apache.kafka.common.security.scram.internals.{ScramCredentialUtils, S
import org.apache.kafka.common.utils.Sanitizer
import org.apache.kafka.server.common.AdminOperationException
import org.apache.kafka.server.config.{ConfigEntityName, ConfigType}
+import org.apache.kafka.server.config.KafkaConfig._
Review Comment:
~For this one and maybe other that import few config from KafkaConfig this
would be nice improvement but some classes like DynamicBrokerConfig for example
include a large set of config. I can update few to use `KAFKA.SOME_CONFIG` if
they need small set of config and leave the others specially that we when we
move them to java we will do this anyway. ~
Ignore this I got the comment wrong. At the moment we can't use KafkaConfig
everywhere as there's another KafkaConfig in scala
##########
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##########
@@ -49,6 +49,7 @@ import
org.apache.kafka.common.security.scram.internals.{ScramCredentialUtils, S
import org.apache.kafka.common.utils.Sanitizer
import org.apache.kafka.server.common.AdminOperationException
import org.apache.kafka.server.config.{ConfigEntityName, ConfigType}
+import org.apache.kafka.server.config.KafkaConfig._
Review Comment:
Also at the moment we can't use `KafkaConfig` everywhere as there's another
`KafkaConfig` in scala
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]