You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/07/29 22:40:33 UTC
[GitHub] [kafka] dielhennr opened a new pull request #11145: KAFKA-13151: KRaft does not support Policies (e.g. AlterConfigPolicy). The server should fail startup if any are configured.
dielhennr opened a new pull request #11145:
URL: https://github.com/apache/kafka/pull/11145
alter.config.policy.class.name and create.topic.policy.class.name are unsupported by KRaft. KRaft servers should fail startup if any of these are configured.
Tested this manually by enabling/disabling the configs statically in a KRaft cluster.
https://issues.apache.org/jira/browse/KAFKA-13151
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dielhennr commented on a change in pull request #11145: KAFKA-13151: KRaft does not support Policies (e.g. AlterConfigPolicy). The server should fail startup if any are configured.
Posted by GitBox <gi...@apache.org>.
dielhennr commented on a change in pull request #11145:
URL: https://github.com/apache/kafka/pull/11145#discussion_r679539020
##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -2008,5 +2008,10 @@ class KafkaConfig(val props: java.util.Map[_, _], doLog: Boolean, dynamicConfigO
require(principalBuilderClass != null, s"${KafkaConfig.PrincipalBuilderClassProp} must be non-null")
require(classOf[KafkaPrincipalSerde].isAssignableFrom(principalBuilderClass),
s"${KafkaConfig.PrincipalBuilderClassProp} must implement KafkaPrincipalSerde")
+
+ if (usesSelfManagedQuorum) {
+ require(getClass(KafkaConfig.AlterConfigPolicyClassNameProp) == null, "alter.config.policy.class.name is not supported in KRaft, please disable.")
Review comment:
yes, sorry I thought I had changed this
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] mumrah commented on a change in pull request #11145: KAFKA-13151: KRaft does not support Policies (e.g. AlterConfigPolicy). The server should fail startup if any are configured.
Posted by GitBox <gi...@apache.org>.
mumrah commented on a change in pull request #11145:
URL: https://github.com/apache/kafka/pull/11145#discussion_r679969846
##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -2008,5 +2008,10 @@ class KafkaConfig(val props: java.util.Map[_, _], doLog: Boolean, dynamicConfigO
require(principalBuilderClass != null, s"${KafkaConfig.PrincipalBuilderClassProp} must be non-null")
require(classOf[KafkaPrincipalSerde].isAssignableFrom(principalBuilderClass),
s"${KafkaConfig.PrincipalBuilderClassProp} must implement KafkaPrincipalSerde")
+
+ if (usesSelfManagedQuorum) {
+ require(getClass(KafkaConfig.AlterConfigPolicyClassNameProp) == null, s"${KafkaConfig.AlterConfigPolicyClassNameProp} is not supported in KRaft, please disable.")
+ require(getClass(KafkaConfig.CreateTopicPolicyClassNameProp) == null, s"${KafkaConfig.CreateTopicPolicyClassNameProp} is not supported in KRaft, please disable.")
Review comment:
How about we just leave off that last bit? I think stating that the config is not supported in KRaft is sufficient. It's up to the user how they want to proceed (either not running KRaft, or disabling the policy config)
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] showuon commented on a change in pull request #11145: KAFKA-13151: KRaft does not support Policies (e.g. AlterConfigPolicy). The server should fail startup if any are configured.
Posted by GitBox <gi...@apache.org>.
showuon commented on a change in pull request #11145:
URL: https://github.com/apache/kafka/pull/11145#discussion_r679768084
##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -2008,5 +2008,10 @@ class KafkaConfig(val props: java.util.Map[_, _], doLog: Boolean, dynamicConfigO
require(principalBuilderClass != null, s"${KafkaConfig.PrincipalBuilderClassProp} must be non-null")
require(classOf[KafkaPrincipalSerde].isAssignableFrom(principalBuilderClass),
s"${KafkaConfig.PrincipalBuilderClassProp} must implement KafkaPrincipalSerde")
+
+ if (usesSelfManagedQuorum) {
+ require(getClass(KafkaConfig.AlterConfigPolicyClassNameProp) == null, s"${KafkaConfig.AlterConfigPolicyClassNameProp} is not supported in KRaft, please disable.")
+ require(getClass(KafkaConfig.CreateTopicPolicyClassNameProp) == null, s"${KafkaConfig.CreateTopicPolicyClassNameProp} is not supported in KRaft, please disable.")
Review comment:
nit: `please disable.` -> `please disable it.` ?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji merged pull request #11145: KAFKA-13151: Disallow policy configs in KRaft
Posted by GitBox <gi...@apache.org>.
hachikuji merged pull request #11145:
URL: https://github.com/apache/kafka/pull/11145
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a change in pull request #11145: KAFKA-13151: KRaft does not support Policies (e.g. AlterConfigPolicy). The server should fail startup if any are configured.
Posted by GitBox <gi...@apache.org>.
hachikuji commented on a change in pull request #11145:
URL: https://github.com/apache/kafka/pull/11145#discussion_r679533321
##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -2008,5 +2008,10 @@ class KafkaConfig(val props: java.util.Map[_, _], doLog: Boolean, dynamicConfigO
require(principalBuilderClass != null, s"${KafkaConfig.PrincipalBuilderClassProp} must be non-null")
require(classOf[KafkaPrincipalSerde].isAssignableFrom(principalBuilderClass),
s"${KafkaConfig.PrincipalBuilderClassProp} must implement KafkaPrincipalSerde")
+
+ if (usesSelfManagedQuorum) {
+ require(getClass(KafkaConfig.AlterConfigPolicyClassNameProp) == null, "alter.config.policy.class.name is not supported in KRaft, please disable.")
Review comment:
nit: can you use `KafkaConfig.AlterConfigPolicyClassNameProp` in the message instead of the explicit config name? Same for the other.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org