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 c4ee122df5 Allow env var substibution for Pinot configs (#10785)
c4ee122df5 is described below
commit c4ee122df56d2fe229fde98cfe5e5643c5c8fc7b
Author: Xiaobing <61892277+klsi...@users.noreply.github.com>
AuthorDate: Wed May 24 10:51:47 2023 -0700
Allow env var substibution for Pinot configs (#10785)
---
.../pinot/spi/env/CommonsConfigurationUtils.java | 56 ++++++++++++++++++++--
.../apache/pinot/spi/env/PinotConfiguration.java | 22 ++++-----
.../apache/pinot/spi/env/PropertyConverter.java | 36 --------------
.../pinot/spi/env/PinotConfigurationTest.java | 52 ++++++++++++++++++++
4 files changed, 115 insertions(+), 51 deletions(-)
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java
index 9bf3575fa2..b8984baf94 100644
---
a/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java
+++
b/pinot-spi/src/main/java/org/apache/pinot/spi/env/CommonsConfigurationUtils.java
@@ -35,6 +35,7 @@ import java.util.stream.StreamSupport;
import org.apache.commons.configuration.Configuration;
import org.apache.commons.configuration.ConfigurationException;
import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.commons.lang3.StringUtils;
/**
@@ -119,8 +120,57 @@ public abstract class CommonsConfigurationUtils {
}
private static Object mapValue(String key, Configuration configuration) {
- return Optional.of(configuration.getStringArray(key)).filter(values ->
values.length > 1).<Object>map(
- values -> Arrays.stream(values).collect(Collectors.joining(",")))
- .orElseGet(() -> configuration.getProperty(key));
+ // For multi-value config, convert it to a single comma connected string
value.
+ // For single-value config, return its raw property, unless it needs
interpolation.
+ return Optional.of(configuration.getStringArray(key)).filter(values ->
values.length > 1)
+ .<Object>map(values ->
Arrays.stream(values).collect(Collectors.joining(","))).orElseGet(() -> {
+ Object rawProperty = configuration.getProperty(key);
+ if (!needInterpolate(rawProperty)) {
+ return rawProperty;
+ }
+ // The string value is converted to the requested type when
accessing it via PinotConfiguration.
+ return configuration.getString(key);
+ });
+ }
+
+ public static boolean needInterpolate(Object rawProperty) {
+ if (rawProperty instanceof String) {
+ String strProperty = (String) rawProperty;
+ // e.g. if config value is '${sys:FOO}', it's replaced by value of
system property FOO; If config value is
+ // '${env:FOO}', it's replaced by value of env var FOO.
+ return StringUtils.isNotEmpty(strProperty) &&
strProperty.startsWith("${") && strProperty.endsWith("}");
+ }
+ return false;
+ }
+
+ @SuppressWarnings("unchecked")
+ public static <T> T interpolate(Configuration configuration, String key, T
defaultValue, Class<T> returnType) {
+ // Different from the generic getProperty() method, those type specific
getters do config interpolation.
+ if (Integer.class.equals(returnType)) {
+ return (T) configuration.getInteger(key, (Integer) defaultValue);
+ } else if (Boolean.class.equals(returnType)) {
+ return (T) configuration.getBoolean(key, (Boolean) defaultValue);
+ } else if (Long.class.equals(returnType)) {
+ return (T) configuration.getLong(key, (Long) defaultValue);
+ } else if (Double.class.equals(returnType)) {
+ return (T) configuration.getDouble(key, (Double) defaultValue);
+ } else {
+ throw new IllegalArgumentException(returnType + " is not a supported
type for conversion.");
+ }
+ }
+
+ @SuppressWarnings("unchecked")
+ public static <T> T convert(Object value, Class<T> returnType) {
+ if (Integer.class.equals(returnType)) {
+ return (T) Integer.valueOf(value.toString());
+ } else if (Boolean.class.equals(returnType)) {
+ return (T) Boolean.valueOf(value.toString());
+ } else if (Long.class.equals(returnType)) {
+ return (T) Long.valueOf(value.toString());
+ } else if (Double.class.equals(returnType)) {
+ return (T) Double.valueOf(value.toString());
+ } else {
+ throw new IllegalArgumentException(returnType + " is not a supported
type for conversion.");
+ }
}
}
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 ab843b3cc0..7bd23f503c 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
@@ -18,7 +18,6 @@
*/
package org.apache.pinot.spi.env;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
@@ -32,7 +31,6 @@ import org.apache.commons.configuration.Configuration;
import org.apache.commons.configuration.ConfigurationException;
import org.apache.commons.configuration.MapConfiguration;
import org.apache.commons.configuration.PropertiesConfiguration;
-import org.apache.commons.lang3.StringUtils;
import org.apache.pinot.spi.ingestion.batch.spec.PinotFSSpec;
import org.apache.pinot.spi.utils.Obfuscator;
@@ -356,15 +354,12 @@ public class PinotConfiguration {
* @return the property String value. Fallback to default value if missing.
*/
public String getProperty(String name, String defaultValue) {
- Object rawProperty = getRawProperty(name, defaultValue);
- if (rawProperty instanceof List) {
- return StringUtils.join(((ArrayList) rawProperty).toArray(), ',');
- } else {
- if (rawProperty == null) {
- return null;
- }
- return rawProperty.toString();
+ String relaxedPropertyName = relaxPropertyName(name);
+ if (!_configuration.containsKey(relaxedPropertyName)) {
+ return defaultValue;
}
+ // Method below calls getStringArray() which can interpolate multi values
properly.
+ return getProperty(name, _configuration);
}
private <T> T getProperty(String name, T defaultValue, Class<T> returnType) {
@@ -372,8 +367,11 @@ public class PinotConfiguration {
if (!_configuration.containsKey(relaxedPropertyName)) {
return defaultValue;
}
-
- return PropertyConverter.convert(getRawProperty(name, defaultValue),
returnType);
+ Object rawProperty = _configuration.getProperty(relaxedPropertyName);
+ if (CommonsConfigurationUtils.needInterpolate(rawProperty)) {
+ return CommonsConfigurationUtils.interpolate(_configuration,
relaxedPropertyName, defaultValue, returnType);
+ }
+ return CommonsConfigurationUtils.convert(rawProperty, returnType);
}
/**
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/env/PropertyConverter.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/env/PropertyConverter.java
deleted file mode 100644
index b2b7ca71a4..0000000000
--- a/pinot-spi/src/main/java/org/apache/pinot/spi/env/PropertyConverter.java
+++ /dev/null
@@ -1,36 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-package org.apache.pinot.spi.env;
-
-public abstract class PropertyConverter {
- @SuppressWarnings("unchecked")
- public static <T> T convert(Object value, Class<T> returnType) {
- if (Integer.class.equals(returnType)) {
- return (T) Integer.valueOf(value.toString());
- } else if (Boolean.class.equals(returnType)) {
- return (T) Boolean.valueOf(value.toString());
- } else if (Long.class.equals(returnType)) {
- return (T) Long.valueOf(value.toString());
- } else if (Double.class.equals(returnType)) {
- return (T) Double.valueOf(value.toString());
- } else {
- throw new IllegalArgumentException(returnType + " is not a supported
type for conversion.");
- }
- }
-}
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 240eb4f31d..0b33d73d71 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
@@ -207,6 +207,58 @@ public class PinotConfigurationTest {
new PinotConfiguration(new PinotFSSpec());
}
+ @Test
+ public void assertPropertyInterpolation() {
+ Map<String, Object> configs = new HashMap<>();
+ configs.put("config.property.1", "${sys:PINOT_CONFIGURATION_TEST_VAR}");
+ PinotConfiguration pinotConfiguration = new PinotConfiguration(configs);
+
+ try {
+ // Env var is not defined, thus interpolation doesn't happen
+ Assert.assertEquals(pinotConfiguration.getProperty("config.property.1"),
"${sys:PINOT_CONFIGURATION_TEST_VAR}");
+
+ // String value
+ System.setProperty("PINOT_CONFIGURATION_TEST_VAR", "val1");
+ Assert.assertEquals(pinotConfiguration.getProperty("config.property.1"),
"val1");
+ Assert.assertEquals(pinotConfiguration.getProperty("config.property.1",
"defaultVal"), "val1");
+ Map<String, Object> properties = pinotConfiguration.toMap();
+ Assert.assertEquals(properties.get("config.property.1"), "val1");
+
+ // Boolean value
+ System.setProperty("PINOT_CONFIGURATION_TEST_VAR", "true");
+ Assert.assertTrue(pinotConfiguration.getProperty("config.property.1",
false));
+ properties = pinotConfiguration.toMap();
+ Assert.assertEquals(properties.get("config.property.1"), "true");
+ Assert.assertTrue(new
PinotConfiguration(properties).getProperty("config.property.1", false));
+
+ // Number value
+ System.setProperty("PINOT_CONFIGURATION_TEST_VAR", "10");
+ Assert.assertEquals(pinotConfiguration.getProperty("config.property.1",
0), 10);
+ properties = pinotConfiguration.toMap();
+ Assert.assertEquals(properties.get("config.property.1"), "10");
+ Assert.assertEquals(new
PinotConfiguration(properties).getProperty("config.property.1", 0), 10);
+
+ // String array, with elements specified as env vars separately.
+ System.setProperty("PINOT_CONFIGURATION_TEST_VAR", "a");
+ System.setProperty("PINOT_CONFIGURATION_TEST_VAR2", "b");
+ configs.put("config.property.1",
"${sys:PINOT_CONFIGURATION_TEST_VAR},${sys:PINOT_CONFIGURATION_TEST_VAR2}");
+ pinotConfiguration = new PinotConfiguration(configs);
+ Assert.assertEquals(pinotConfiguration.getProperty("config.property.1",
Arrays.asList()),
+ Arrays.asList("a", "b"));
+ Assert.assertEquals(pinotConfiguration.getProperty("config.property.1",
"val1"), "a,b");
+ Assert.assertEquals(pinotConfiguration.getProperty("config.property.1"),
"a,b");
+ properties = pinotConfiguration.toMap();
+ Assert.assertEquals(properties.get("config.property.1"), "a,b");
+ Assert.assertEquals(new
PinotConfiguration(properties).getProperty("config.property.1",
Arrays.asList()),
+ Arrays.asList("a", "b"));
+ Assert.assertEquals(new
PinotConfiguration(properties).getProperty("config.property.1", "val1"), "a,b");
+ Assert.assertEquals(new
PinotConfiguration(properties).getProperty("config.property.1"), "a,b");
+ } finally {
+ System.clearProperty("PINOT_CONFIGURATION_TEST_VAR");
+ System.clearProperty("PINOT_CONFIGURATION_TEST_VAR2");
+ }
+ }
+
private void copyClasspathResource(String classpathResource, String target)
throws IOException {
try (InputStream inputStream =
PinotConfigurationTest.class.getResourceAsStream(classpathResource)) {
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
