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