Jackie-Jiang commented on a change in pull request #6515:
URL: https://github.com/apache/incubator-pinot/pull/6515#discussion_r568175015
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java
##########
@@ -112,6 +112,12 @@ private ListenerConfigUtil() {
listeners.addAll(buildListenerConfigs(brokerConf, "pinot.broker.client",
tlsDefaults));
+ // support legacy behavior < 0.7.0
+ if (listeners.isEmpty()) {
Review comment:
You can build the ListenerConfigs from the config first, then append the
HTTP config if the ListenerConfigs is empty?
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/util/ListenerConfigUtil.java
##########
@@ -128,6 +134,12 @@ private ListenerConfigUtil() {
listeners.addAll(buildListenerConfigs(serverConf, "pinot.server.adminapi",
tlsDefaults));
+ // support legacy behavior < 0.7.0
+ if (listeners.isEmpty()) {
Review comment:
Same here
##########
File path:
pinot-broker/src/main/java/org/apache/pinot/broker/broker/helix/HelixBrokerStarter.java
##########
@@ -133,9 +133,8 @@ public HelixBrokerStarter(PinotConfiguration brokerConf,
String clusterName, Str
private int inferPort() {
return
Optional.ofNullable(_brokerConf.getProperty(Helix.KEY_OF_BROKER_QUERY_PORT)).map(Integer::parseInt)
- .orElseGet(() ->
_listenerConfigs.stream().findFirst().map(ListenerConfig::getPort).orElseThrow(()
->
- new IllegalStateException(String.format("Requires at least one
ingress config or '%s'",
- Helix.KEY_OF_BROKER_QUERY_PORT))));
+ .orElseGet(() ->
_listenerConfigs.stream().findFirst().map(ListenerConfig::getPort)
Review comment:
With the current way of extracting the listener config, we should be
able to always use the first one within the listener config?
##########
File path:
pinot-controller/src/main/java/org/apache/pinot/controller/ControllerStarter.java
##########
@@ -197,7 +197,7 @@ private int inferPort() {
return
Optional.ofNullable(_config.getControllerPort()).map(Integer::parseInt)
.orElseGet(() ->
_listenerConfigs.stream().findFirst().map(ListenerConfig::getPort).orElseThrow(()
->
new IllegalStateException(String.format("Requires at least one
ingress config or '%s'",
- ControllerConf.CONTROLLER_PORT))));
+ CommonConstants.Helix.KEY_OF_BROKER_QUERY_PORT))));
Review comment:
Revert 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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
