This is an automated email from the ASF dual-hosted git repository.

jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 49698174ee do not fail on duplicate relaxed vars (#13214)
49698174ee is described below

commit 49698174eea43fbaec331d298da8031db4e4ef03
Author: Johan Adami <4760722+jadam...@users.noreply.github.com>
AuthorDate: Mon Jun 10 15:01:42 2024 -0400

    do not fail on duplicate relaxed vars (#13214)
---
 .../apache/pinot/spi/env/PinotConfiguration.java   | 10 ++++++-
 .../pinot/spi/env/PinotConfigurationTest.java      | 33 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git 
a/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java 
b/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java
index 72be975544..8615f61923 100644
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java
+++ b/pinot-spi/src/main/java/org/apache/pinot/spi/env/PinotConfiguration.java
@@ -233,7 +233,15 @@ public class PinotConfiguration {
 
   private static Map<String, String> relaxEnvironmentVariables(Map<String, 
String> environmentVariables) {
     return environmentVariables.entrySet().stream()
-        .collect(Collectors.toMap(PinotConfiguration::relaxEnvVarName, 
Entry::getValue));
+        // Sort by key to ensure choosing a consistent env var if there are 
collisions
+        .sorted(Entry.comparingByKey())
+        .collect(Collectors.toMap(
+            PinotConfiguration::relaxEnvVarName,
+            Entry::getValue,
+            // Nothing prevents users from setting env variables like foo_bar 
and foo.bar
+            // This should not prevent Pinot from starting.
+            (existingValue, newValue) -> existingValue
+        ));
   }
 
   private static String relaxEnvVarName(Entry<String, String> envVarEntry) {
diff --git 
a/pinot-spi/src/test/java/org/apache/pinot/spi/env/PinotConfigurationTest.java 
b/pinot-spi/src/test/java/org/apache/pinot/spi/env/PinotConfigurationTest.java
index 1c810dc6ef..02000b746d 100644
--- 
a/pinot-spi/src/test/java/org/apache/pinot/spi/env/PinotConfigurationTest.java
+++ 
b/pinot-spi/src/test/java/org/apache/pinot/spi/env/PinotConfigurationTest.java
@@ -187,6 +187,39 @@ public class PinotConfigurationTest {
     Assert.assertEquals(configuration.getProperty("pinot.controller.host"), 
"test-host");
   }
 
+  @Test
+  public void assertHandlesDuplicateRelaxedKeys()
+      throws IOException {
+    Map<String, Object> baseProperties = new HashMap<>();
+    Map<String, String> mockedEnvironmentVariables = new HashMap<>();
+
+    String configFile = File.createTempFile("pinot-configuration-test-4", 
".properties").getAbsolutePath();
+
+    baseProperties.put("server.host", "ENV_SERVER_HOST");
+    baseProperties.put("dynamic.env.config", "server.host");
+
+    mockedEnvironmentVariables.put("ENV_SERVER_HOST", "test-server-host-1");
+    mockedEnvironmentVariables.put("ENV.SERVER.HOST", "test-server-host-2");
+    mockedEnvironmentVariables.put("ENV.SERVER_HOST", "test-server-host-3");
+    mockedEnvironmentVariables.put("ENV_SERVER.HOST", "test-server-host-4");
+
+    mockedEnvironmentVariables.put("ENV_VAR_HOST", "test-host-1");
+    mockedEnvironmentVariables.put("ENV.VAR_HOST", "test-host-2");
+    mockedEnvironmentVariables.put("env_var_host", "test-host-3");
+    mockedEnvironmentVariables.put("env_var.host", "test-host-4");
+
+    // config.paths sorts before config_paths, so we should get the right 
config
+    mockedEnvironmentVariables.put("config.paths", 
"classpath:/pinot-configuration-4.properties");
+    mockedEnvironmentVariables.put("config_paths", 
"classpath:/does-not-exist-configuration.properties");
+    copyClasspathResource("/pinot-configuration-4.properties", configFile);
+
+    PinotConfiguration configuration = new PinotConfiguration(baseProperties, 
mockedEnvironmentVariables);
+
+    // These are taken from raw and not relaxed values
+    Assert.assertEquals(configuration.getProperty("server.host"), 
"test-server-host-1");
+    Assert.assertEquals(configuration.getProperty("pinot.controller.host"), 
"test-host-1");
+  }
+
   @Test(expectedExceptions = IllegalArgumentException.class)
   public void assertInvalidConfigPathBehavior() {
     Map<String, Object> baseProperties = new HashMap<>();


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to