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

Reply via email to