This is an automated email from the ASF dual-hosted git repository.
zuston pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-uniffle.git
The following commit(s) were added to refs/heads/master by this push:
new 24a51954 [#841] feat(config): Support deprecated and fallback keys for
ConfigOptions (#843)
24a51954 is described below
commit 24a51954aaa72633fb6640a7195000adbc9034b1
Author: Junfan Zhang <[email protected]>
AuthorDate: Thu Apr 27 13:19:48 2023 +0800
[#841] feat(config): Support deprecated and fallback keys for ConfigOptions
(#843)
### What changes were proposed in this pull request?
Support deprecated and fallback keys for ConfigOptions
### Why are the changes needed?
#841 . To achieve better compatibility, it's time to support deprecated
keys for ConfigOptions .
After this PR, the configOption will have the fallback and deprecated keys.
Like this
```java
final ConfigOption<Integer> intConfig = ConfigOptions
.key("rss.main.key")
.intType()
.defaultValue(100)
.withFallbackKeys("rss.fallback.key1")
.withDeprecatedKeys("rss.deprecated.key1");
```
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
1. Existing UTs
2. Newly UTs
---
.../apache/uniffle/common/config/ConfigOption.java | 59 +++++++++++-
.../apache/uniffle/common/config/FallbackKey.java | 59 ++++++++++++
.../org/apache/uniffle/common/config/RssConf.java | 16 ++++
.../uniffle/common/config/ConfigOptionTest.java | 106 +++++++++++++++++++++
4 files changed, 239 insertions(+), 1 deletion(-)
diff --git
a/common/src/main/java/org/apache/uniffle/common/config/ConfigOption.java
b/common/src/main/java/org/apache/uniffle/common/config/ConfigOption.java
index 7f420d71..fac73fbe 100644
--- a/common/src/main/java/org/apache/uniffle/common/config/ConfigOption.java
+++ b/common/src/main/java/org/apache/uniffle/common/config/ConfigOption.java
@@ -17,8 +17,11 @@
package org.apache.uniffle.common.config;
+import java.util.Arrays;
+import java.util.Collections;
import java.util.Objects;
import java.util.function.Function;
+import java.util.stream.Stream;
/**
* A {@code ConfigOption} describes a configuration parameter. It encapsulates
@@ -31,6 +34,7 @@ import java.util.function.Function;
* @param <T> The type of value associated with the configuration option.
*/
public class ConfigOption<T> {
+ static final FallbackKey[] EMPTY = new FallbackKey[0];
static final String EMPTY_DESCRIPTION = "";
/**
@@ -58,6 +62,9 @@ public class ConfigOption<T> {
*/
private final Function<Object, T> converter;
+ /** The list of deprecated keys, in the order to be checked. */
+ private final FallbackKey[] fallbackKeys;
+
/**
* Creates a new config option with fallback keys.
*
@@ -72,12 +79,14 @@ public class ConfigOption<T> {
Class<?> clazz,
String description,
T defaultValue,
- Function<Object, T> converter) {
+ Function<Object, T> converter,
+ FallbackKey... fallbackKeys) {
this.key = Objects.requireNonNull(key);
this.description = description;
this.defaultValue = defaultValue;
this.clazz = Objects.requireNonNull(clazz);
this.converter = Objects.requireNonNull(converter);
+ this.fallbackKeys = fallbackKeys == null || fallbackKeys.length == 0 ?
EMPTY : fallbackKeys;
}
/**
@@ -91,6 +100,45 @@ public class ConfigOption<T> {
return new ConfigOption<>(key, clazz, description, defaultValue,
converter);
}
+ /**
+ * Creates a new config option, using this option's key and default value,
and adding the given
+ * fallback keys.
+ *
+ * @param fallbackKeys The fallback keys, in the order in which they should
be checked.
+ * @return A new config options, with the given fallback keys.
+ */
+ public ConfigOption<T> withFallbackKeys(String... fallbackKeys) {
+ final Stream<FallbackKey> newFallbackKeys =
+ Arrays.stream(fallbackKeys).map(FallbackKey::createFallbackKey);
+ final Stream<FallbackKey> currentAlternativeKeys =
Arrays.stream(this.fallbackKeys);
+
+ // put fallback keys first so that they are prioritized
+ final FallbackKey[] mergedAlternativeKeys =
+ Stream.concat(newFallbackKeys,
currentAlternativeKeys).toArray(FallbackKey[]::new);
+ return new ConfigOption<>(
+ key, clazz, description, defaultValue, converter,
mergedAlternativeKeys);
+ }
+
+ /**
+ * Creates a new config option, using this option's key and default value,
and adding the given
+ * deprecated keys.
+ *
+ * @param deprecatedKeys The deprecated keys, in the order in which they
should be checked.
+ * @return A new config options, with the given deprecated keys.
+ */
+ public ConfigOption<T> withDeprecatedKeys(final String... deprecatedKeys) {
+ final Stream<FallbackKey> newDeprecatedKeys =
+ Arrays.stream(deprecatedKeys).map(FallbackKey::createDeprecatedKey);
+ final Stream<FallbackKey> currentAlternativeKeys =
Arrays.stream(this.fallbackKeys);
+
+ // put deprecated keys last so that they are de-prioritized
+ final FallbackKey[] mergedAlternativeKeys =
+ Stream.concat(currentAlternativeKeys, newDeprecatedKeys)
+ .toArray(FallbackKey[]::new);
+ return new ConfigOption<>(
+ key, clazz, description, defaultValue, converter,
mergedAlternativeKeys);
+ }
+
// ------------------------------------------------------------------------
/**
@@ -120,6 +168,15 @@ public class ConfigOption<T> {
return defaultValue;
}
+ /**
+ * Gets the fallback keys, in the order to be checked.
+ *
+ * @return The option's fallback keys.
+ */
+ public Iterable<FallbackKey> fallbackKeys() {
+ return (fallbackKeys == EMPTY) ? Collections.emptyList() :
Arrays.asList(fallbackKeys);
+ }
+
/**
* Returns the class of value.
*
diff --git
a/common/src/main/java/org/apache/uniffle/common/config/FallbackKey.java
b/common/src/main/java/org/apache/uniffle/common/config/FallbackKey.java
new file mode 100644
index 00000000..17a86115
--- /dev/null
+++ b/common/src/main/java/org/apache/uniffle/common/config/FallbackKey.java
@@ -0,0 +1,59 @@
+/*
+ * 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.uniffle.common.config;
+
+/** A key with FallbackKeys will fall back to the FallbackKeys if it itself is
not configured. */
+public class FallbackKey {
+
+ // -------------------------
+ // Factory methods
+ // -------------------------
+
+ static FallbackKey createFallbackKey(String key) {
+ return new FallbackKey(key, false);
+ }
+
+ static FallbackKey createDeprecatedKey(String key) {
+ return new FallbackKey(key, true);
+ }
+
+ // ------------------------------------------------------------------------
+
+ private final String key;
+
+ private final boolean isDeprecated;
+
+ public String getKey() {
+ return key;
+ }
+
+ public boolean isDeprecated() {
+ return isDeprecated;
+ }
+
+ private FallbackKey(String key, boolean isDeprecated) {
+ this.key = key;
+ this.isDeprecated = isDeprecated;
+ }
+
+ @Override
+ public String toString() {
+ return String.format("{key=%s, isDeprecated=%s}", key, isDeprecated);
+ }
+}
diff --git a/common/src/main/java/org/apache/uniffle/common/config/RssConf.java
b/common/src/main/java/org/apache/uniffle/common/config/RssConf.java
index 1f9da666..d15e642f 100644
--- a/common/src/main/java/org/apache/uniffle/common/config/RssConf.java
+++ b/common/src/main/java/org/apache/uniffle/common/config/RssConf.java
@@ -18,6 +18,7 @@
package org.apache.uniffle.common.config;
import java.util.Arrays;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Optional;
@@ -569,8 +570,23 @@ public class RssConf implements Cloneable {
.orElseGet(option::defaultValue);
}
+ private Optional<Object> geRawValFromFallbackKeys(Iterator<FallbackKey>
iter) {
+ if (iter.hasNext()) {
+ FallbackKey key = iter.next();
+ Optional<Object> rawVal = getRawValue(key.getKey());
+ if (!rawVal.isPresent()) {
+ return geRawValFromFallbackKeys(iter);
+ }
+ return rawVal;
+ }
+ return Optional.empty();
+ }
+
public <T> Optional<T> getOptional(ConfigOption<T> option) {
Optional<Object> rawValue = getRawValueFromOption(option);
+ if (!rawValue.isPresent()) {
+ rawValue = geRawValFromFallbackKeys(option.fallbackKeys().iterator());
+ }
Class<?> clazz = option.getClazz();
Optional<T> value = rawValue.map(v -> option.convertValue(v, clazz));
return value;
diff --git
a/common/src/test/java/org/apache/uniffle/common/config/ConfigOptionTest.java
b/common/src/test/java/org/apache/uniffle/common/config/ConfigOptionTest.java
index 996fb6ea..7340715c 100644
---
a/common/src/test/java/org/apache/uniffle/common/config/ConfigOptionTest.java
+++
b/common/src/test/java/org/apache/uniffle/common/config/ConfigOptionTest.java
@@ -32,6 +32,112 @@ import static org.junit.jupiter.api.Assertions.fail;
public class ConfigOptionTest {
+ @Test
+ public void testDeprecatedAndFallbackKeys() {
+ final ConfigOption<Integer> intConfig = ConfigOptions
+ .key("rss.main.key")
+ .intType().defaultValue(100)
+ .withFallbackKeys("rss.fallback.key1")
+ .withDeprecatedKeys("rss.deprecated.key1");
+
+ // case1
+ RssConf conf = new RssBaseConf();
+ assertEquals(100, conf.get(intConfig));
+
+ // case2
+ conf = new RssBaseConf();
+ conf.setString("rss.fallback.key1", "12");
+ conf.setString("rss.deprecated.key1", "13");
+ assertEquals(12, conf.get(intConfig));
+
+ // case3
+ conf = new RssBaseConf();
+ conf.setString("rss.deprecated.key1", "13");
+ assertEquals(13, conf.get(intConfig));
+
+ // case4
+ conf = new RssBaseConf();
+ conf.setString("rss.fallback.key1", "12");
+ assertEquals(12, conf.get(intConfig));
+
+ // case5
+ conf = new RssBaseConf();
+ conf.setString(intConfig.key(), "999");
+ conf.setString("rss.fallback.key1", "12");
+ conf.setString("rss.deprecated.key1", "13");
+ assertEquals(999, conf.get(intConfig));
+ }
+
+ @Test
+ public void testFallbackKeys() {
+ final ConfigOption<Integer> config1 = ConfigOptions
+ .key("rss.key.1")
+ .intType()
+ .defaultValue(1);
+
+ final ConfigOption<Integer> config2 = ConfigOptions
+ .key("rss.key.2")
+ .intType()
+ .defaultValue(2);
+
+ final ConfigOption<Integer> config3 = ConfigOptions
+ .key("rss.key.3")
+ .intType()
+ .defaultValue(9999)
+ .withFallbackKeys(config1.key(), config2.key());
+
+ // case1
+ RssConf conf = new RssBaseConf();
+ conf.setString(config1.key(), "10");
+ assertEquals(10, conf.get(config3));
+
+ // case2
+ conf.setString(config3.key(), "1111");
+ assertEquals(1111, conf.get(config3));
+
+ // case3
+ conf = new RssBaseConf();
+ assertEquals(9999, conf.get(config3));
+ }
+
+ @Test
+ public void testDeprecatedKeys() {
+ final ConfigOption<Integer> intConfig = ConfigOptions
+ .key("rss.key")
+ .intType()
+ .defaultValue(100)
+ .withDeprecatedKeys("rss.s1", "rss.s2", "rss.s3");
+
+ // case 1
+ RssConf conf = new RssBaseConf();
+ conf.setString("rss.s1", "10");
+ int val = conf.get(intConfig);
+ assertEquals(10, val);
+
+ // case 2
+ conf = new RssBaseConf();
+ conf.setString("rss.s1", "1");
+ conf.setString("rss.s2", "2");
+ conf.setString("rss.s3", "3");
+ assertEquals(1, conf.get(intConfig));
+
+ // case 3
+ conf = new RssBaseConf();
+ conf.setString("rss.s3", "3");
+ conf.setString("rss.s2", "2");
+ assertEquals(2, conf.get(intConfig));
+
+ // case 4
+ conf = new RssBaseConf();
+ conf.setString(intConfig.key(), "25");
+ conf.setString("rss.s3", "3");
+ assertEquals(25, conf.get(intConfig));
+
+ // case 5
+ conf = new RssBaseConf();
+ assertEquals(100, conf.get(intConfig));
+ }
+
@Test
public void testSetKVWithStringTypeDirectly() {
final ConfigOption<Integer> intConfig = ConfigOptions