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/04/07 11:01:33 UTC

[GitHub] [kafka] chia7712 commented on a change in pull request #10437: MINOR: Remove deprecated `host.name`, `advertised.host.name`, `port` and `advertised.port` for 3.0

chia7712 commented on a change in pull request #10437:
URL: https://github.com/apache/kafka/pull/10437#discussion_r608500887



##########
File path: connect/runtime/src/test/java/org/apache/kafka/connect/util/clusters/EmbeddedKafkaCluster.java
##########
@@ -232,7 +232,9 @@ public String bootstrapServers() {
     }
 
     public String address(KafkaServer server) {
-        return server.config().hostName() + ":" + server.boundPort(listenerName);
+        final String hostName = JavaConverters.asJavaCollection(server.config().advertisedListeners())

Review comment:
       Maybe we can move this method to `KafkaConfig`. It is useful for testing (connect and streams)

##########
File path: core/src/main/scala/kafka/server/KafkaConfig.scala
##########
@@ -1838,8 +1805,6 @@ class KafkaConfig(val props: java.util.Map[_, _], doLog: Boolean, dynamicConfigO
     val advertisedListenersProp = getString(KafkaConfig.AdvertisedListenersProp)
     if (advertisedListenersProp != null)
       CoreUtils.listenerListToEndPoints(advertisedListenersProp, listenerSecurityProtocolMap, requireDistinctPorts=false)

Review comment:
       the docs of this method need to be updated.

##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/utils/KafkaEmbedded.java
##########
@@ -108,8 +107,10 @@ private Properties effectiveConfigFrom(final Properties initialConfig) {
     @SuppressWarnings("WeakerAccess")
     public String brokerList() {
         final Object listenerConfig = effectiveConfig.get(KafkaConfig$.MODULE$.InterBrokerListenerNameProp());

Review comment:
       not sure why it tried to get inter listeners. The return value of this method is used for producer/consumer.

##########
File path: streams/src/test/java/org/apache/kafka/streams/integration/utils/EmbeddedKafkaCluster.java
##########
@@ -101,7 +101,7 @@ public void start() throws IOException {
 
         for (int i = 0; i < brokers.length; i++) {
             brokerConfig.put(KafkaConfig$.MODULE$.BrokerIdProp(), i);
-            log.debug("Starting a Kafka instance on port {} ...", brokerConfig.get(KafkaConfig$.MODULE$.PortProp()));
+            log.debug("Starting a Kafka instance on {} ...", brokerConfig.get(KafkaConfig$.MODULE$.ListenersProp()));

Review comment:
       The default port is 0 so the return value of `brokerConfig.get(KafkaConfig$.MODULE$.ListenersProp())` is 0, right? If so, it is weird to log 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org