xiangfu0 commented on code in PR #12307: URL: https://github.com/apache/pinot/pull/12307#discussion_r1462287971
########## pinot-spi/src/main/java/org/apache/pinot/spi/utils/CommonConstants.java: ########## @@ -331,7 +331,6 @@ public static class Broker { // Broker config indicating the maximum length of the serialized response per server for a query. public static final String CONFIG_OF_MAX_SERVER_RESPONSE_SIZE_BYTES = "pinot.broker.max.server.response.size.bytes"; Review Comment: revert change on this file ########## pinot-spi/src/main/java/org/apache/pinot/spi/env/Environment.java: ########## @@ -23,4 +23,4 @@ public interface Environment { Map<String, String> getEnvironmentVariables(); -} +} Review Comment: revert change on this file ########## pinot-controller/src/main/java/org/apache/pinot/controller/api/access/BasicAuthAccessControlFactory.java: ########## @@ -45,6 +45,7 @@ * </pre> */ public class BasicAuthAccessControlFactory implements AccessControlFactory { + Review Comment: revert change on this file ########## pinot-spi/src/main/java/org/apache/pinot/spi/env/SystemEnvironment.java: ########## @@ -27,4 +27,4 @@ public class SystemEnvironment implements Environment { public Map<String, String> getEnvironmentVariables() { return System.getenv(); } -} +} Review Comment: revert change on this file ########## pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java: ########## @@ -201,16 +200,16 @@ private static Configuration loadProperties(String configPath) { private static Map<String, Object> relaxConfigurationKeys(Configuration configuration) { return CommonsConfigurationUtils.getKeysStream(configuration).distinct() - .collect(Collectors.toMap(PinotConfiguration::relaxPropertyName, key -> configuration.getProperty(key))); + .collect(Collectors.toMap(PinotConfiguration::relaxPropertyName, configuration::getProperty)); } private static Map<String, String> relaxEnvironmentVariables(Map<String, String> environmentVariables) { - return environmentVariables.entrySet().stream().filter(entry -> entry.getKey().startsWith("PINOT_")) + return environmentVariables.entrySet().stream().filter(entry -> entry.getKey().startsWith(ENV_PREFIX)) Review Comment: This will break all existing changes right? I would suggest to keep current code rename it to legacy, so current user won't fail for using `PINOT_` prefix. Then add new code to override based on the new prefix. ########## pinot-minion/src/main/java/org/apache/pinot/minion/BaseMinionStarter.java: ########## @@ -203,6 +203,7 @@ public void start() } // initialize authentication + LOGGER.info("Initializing AuthProvider"); Review Comment: revert change on this file -- 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: commits-unsubscr...@pinot.apache.org 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