hachikuji commented on a change in pull request #9934:
URL: https://github.com/apache/kafka/pull/9934#discussion_r562114271
##########
File path: raft/README.md
##########
@@ -12,17 +12,14 @@ Below we describe the details to set this up.
bin/test-raft-server-start.sh config/raft.properties
### Run Multi Node Quorum ###
-Create 3 separate raft quorum properties as the following
Review comment:
Yeah, you're right. That's pretty annoying. Let me see if I can do
anything about it.
##########
File path: clients/src/main/java/org/apache/kafka/common/protocol/ApiKeys.java
##########
@@ -116,8 +116,8 @@
/** indicates the minimum required inter broker magic required to support
the API */
public final byte minRequiredInterBrokerMagic;
- /** indicates whether the API is enabled and should be exposed in
ApiVersions **/
- public final boolean isEnabled;
+ /** indicates whether this is an API which is only exposed by the KIP-500
controller **/
+ public final boolean isControllerOnlyApi;
Review comment:
Filed this JIRA: https://issues.apache.org/jira/browse/KAFKA-12232.
##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -1876,5 +1874,9 @@ class KafkaConfig(val props: java.util.Map[_, _], doLog:
Boolean, dynamicConfigO
s"${KafkaConfig.FailedAuthenticationDelayMsProp}=$failedAuthenticationDelayMs
should always be less than" +
s" ${KafkaConfig.ConnectionsMaxIdleMsProp}=$connectionsMaxIdleMs to
prevent failed" +
s" authentication responses from timing out")
+
+ if (requiresZookeeper && zkConnect == null) {
Review comment:
Ack. Will add a test case.
##########
File path: core/src/main/scala/kafka/Kafka.scala
##########
@@ -65,11 +65,12 @@ object Kafka extends Logging {
private def buildServer(props: Properties): Server = {
val config = KafkaConfig.fromProps(props, false)
- if (config.processRoles.isEmpty) {
+ if (config.requiresZookeeper) {
Review comment:
Hmm `requiresZookeeper` seemed more explicit. Using `processRoles`
seemed a little more obscure.
##########
File path: core/src/main/scala/kafka/Kafka.scala
##########
@@ -65,11 +65,12 @@ object Kafka extends Logging {
private def buildServer(props: Properties): Server = {
val config = KafkaConfig.fromProps(props, false)
- if (config.processRoles.isEmpty) {
+ if (config.requiresZookeeper) {
Review comment:
Hmm `requiresZookeeper` seemed more explicit. I thought using
`processRoles` was a little obscure.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
