[GitHub] [kafka] rhauch commented on a change in pull request #10841: KAFKA-12482 Remove deprecated rest.host.name and rest.port configs

[GitBox]

rhauch commented on a change in pull request #10841:
URL: https://github.com/apache/kafka/pull/10841#discussion_r647588529



##########
File path: 
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/WorkerConfig.java
##########
@@ -304,8 +285,6 @@ protected static ConfigDef baseConfigDef() {
                         Importance.LOW, OFFSET_COMMIT_INTERVAL_MS_DOC)
                 .define(OFFSET_COMMIT_TIMEOUT_MS_CONFIG, Type.LONG, 
OFFSET_COMMIT_TIMEOUT_MS_DEFAULT,
                         Importance.LOW, OFFSET_COMMIT_TIMEOUT_MS_DOC)
-                .define(REST_HOST_NAME_CONFIG, Type.STRING, null, 
Importance.LOW, REST_HOST_NAME_DOC)
-                .define(REST_PORT_CONFIG, Type.INT, REST_PORT_DEFAULT, 
Importance.LOW, REST_PORT_DOC)
                 .define(LISTENERS_CONFIG, Type.LIST, null, Importance.LOW, 
LISTENERS_DOC)

Review comment:
       Should we add more validation here, via a custom validator, to ensure 
that at least one listener is required?

##########
File path: 
connect/runtime/src/main/java/org/apache/kafka/connect/runtime/rest/RestServer.java
##########
@@ -105,32 +105,19 @@ public RestServer(WorkerConfig config) {
         createConnectors(listeners, adminListeners);
     }
 
-    @SuppressWarnings("deprecation")
-    List<String> parseListeners() {
-        List<String> listeners = config.getList(WorkerConfig.LISTENERS_CONFIG);
-        if (listeners == null || listeners.size() == 0) {
-            String hostname = 
config.getString(WorkerConfig.REST_HOST_NAME_CONFIG);
-
-            if (hostname == null)
-                hostname = "";
-
-            listeners = Collections.singletonList(String.format("%s://%s:%d", 
PROTOCOL_HTTP, hostname, config.getInt(WorkerConfig.REST_PORT_CONFIG)));
-        }
-
-        return listeners;
-    }
-
     /**
      * Adds Jetty connector for each configured listener
      */
     public void createConnectors(List<String> listeners, List<String> 
adminListeners) {
         List<Connector> connectors = new ArrayList<>();
 
-        for (String listener : listeners) {
-            if (!listener.isEmpty()) {
-                Connector connector = createConnector(listener);
-                connectors.add(connector);
-                log.info("Added connector for {}", listener);
+        if (listeners != null && !listeners.isEmpty()) {
+            for (String listener : listeners) {
+                if (!listener.isEmpty()) {
+                    Connector connector = createConnector(listener);
+                    connectors.add(connector);
+                    log.info("Added connector for {}", listener);
+                }

Review comment:
       Are we just relying upon worker config validation to ensure that at 
least one listener is required? If so, then do we need the outer condition? If 
not, then should we add those checks via validators so configuration errors are 
checked immediately upon startup?




-- 
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