Repository: flink Updated Branches: refs/heads/master 8d6961bd9 -> ac9156ade
[FLINK-8743][docs] Allow overriding documented default This closes #5822. Project: http://git-wip-us.apache.org/repos/asf/flink/repo Commit: http://git-wip-us.apache.org/repos/asf/flink/commit/c7d59105 Tree: http://git-wip-us.apache.org/repos/asf/flink/tree/c7d59105 Diff: http://git-wip-us.apache.org/repos/asf/flink/diff/c7d59105 Branch: refs/heads/master Commit: c7d59105c0dfae172db8d70820077d81040b3f6d Parents: 678ab6c Author: zentol <[email protected]> Authored: Thu Apr 5 12:42:42 2018 +0200 Committer: zentol <[email protected]> Committed: Wed May 2 15:18:06 2018 +0200 ---------------------------------------------------------------------- .../_includes/generated/core_configuration.html | 2 +- docs/_includes/generated/web_configuration.html | 2 +- .../flink/annotation/docs/Documentation.java | 45 +++++++++++++ .../apache/flink/configuration/CoreOptions.java | 2 + .../apache/flink/configuration/WebOptions.java | 2 + .../ConfigOptionsDocGenerator.java | 69 ++++++++++++-------- .../ConfigOptionsDocGeneratorTest.java | 44 +++++++++++++ 7 files changed, 135 insertions(+), 31 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/flink/blob/c7d59105/docs/_includes/generated/core_configuration.html ---------------------------------------------------------------------- diff --git a/docs/_includes/generated/core_configuration.html b/docs/_includes/generated/core_configuration.html index 606fd5b..5e29cb9 100644 --- a/docs/_includes/generated/core_configuration.html +++ b/docs/_includes/generated/core_configuration.html @@ -24,7 +24,7 @@ </tr> <tr> <td><h5>io.tmp.dirs</h5></td> - <td style="word-wrap: break-word;">(none)</td> + <td style="word-wrap: break-word;">System.getProperty("java.io.tmpdir")</td> <td></td> </tr> <tr> http://git-wip-us.apache.org/repos/asf/flink/blob/c7d59105/docs/_includes/generated/web_configuration.html ---------------------------------------------------------------------- diff --git a/docs/_includes/generated/web_configuration.html b/docs/_includes/generated/web_configuration.html index 23bf5ab..d4f5a7e 100644 --- a/docs/_includes/generated/web_configuration.html +++ b/docs/_includes/generated/web_configuration.html @@ -79,7 +79,7 @@ </tr> <tr> <td><h5>web.tmpdir</h5></td> - <td style="word-wrap: break-word;">(none)</td> + <td style="word-wrap: break-word;">System.getProperty("java.io.tmpdir")</td> <td></td> </tr> <tr> http://git-wip-us.apache.org/repos/asf/flink/blob/c7d59105/flink-annotations/src/main/java/org/apache/flink/annotation/docs/Documentation.java ---------------------------------------------------------------------- diff --git a/flink-annotations/src/main/java/org/apache/flink/annotation/docs/Documentation.java b/flink-annotations/src/main/java/org/apache/flink/annotation/docs/Documentation.java new file mode 100644 index 0000000..4e424c7 --- /dev/null +++ b/flink-annotations/src/main/java/org/apache/flink/annotation/docs/Documentation.java @@ -0,0 +1,45 @@ +/* + * 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.flink.annotation.docs; + +import org.apache.flink.annotation.Internal; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Collection of annotations to modify the behavior of the documentation generators. + */ +public final class Documentation { + + /** + * Annotation used on config option fields to override the documented default. + */ + @Target(ElementType.FIELD) + @Retention(RetentionPolicy.RUNTIME) + @Internal + public @interface OverrideDefault { + String value(); + } + + private Documentation(){ + } +} http://git-wip-us.apache.org/repos/asf/flink/blob/c7d59105/flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java ---------------------------------------------------------------------- diff --git a/flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java b/flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java index 343e2d2..42c1c9c 100644 --- a/flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java +++ b/flink-core/src/main/java/org/apache/flink/configuration/CoreOptions.java @@ -21,6 +21,7 @@ package org.apache.flink.configuration; import org.apache.flink.annotation.PublicEvolving; import org.apache.flink.annotation.docs.ConfigGroup; import org.apache.flink.annotation.docs.ConfigGroups; +import org.apache.flink.annotation.docs.Documentation; import static org.apache.flink.configuration.ConfigOptions.key; @@ -180,6 +181,7 @@ public class CoreOptions { * The config parameter defining the directories for temporary files, separated by * ",", "|", or the system's {@link java.io.File#pathSeparator}. */ + @Documentation.OverrideDefault("System.getProperty(\"java.io.tmpdir\")") public static final ConfigOption<String> TMP_DIRS = key("io.tmp.dirs") .defaultValue(System.getProperty("java.io.tmpdir")) http://git-wip-us.apache.org/repos/asf/flink/blob/c7d59105/flink-core/src/main/java/org/apache/flink/configuration/WebOptions.java ---------------------------------------------------------------------- diff --git a/flink-core/src/main/java/org/apache/flink/configuration/WebOptions.java b/flink-core/src/main/java/org/apache/flink/configuration/WebOptions.java index 7eea17d..ff640cc 100644 --- a/flink-core/src/main/java/org/apache/flink/configuration/WebOptions.java +++ b/flink-core/src/main/java/org/apache/flink/configuration/WebOptions.java @@ -19,6 +19,7 @@ package org.apache.flink.configuration; import org.apache.flink.annotation.PublicEvolving; +import org.apache.flink.annotation.docs.Documentation; import static org.apache.flink.configuration.ConfigOptions.key; @@ -72,6 +73,7 @@ public class WebOptions { /** * The config parameter defining the flink web directory to be used by the webmonitor. */ + @Documentation.OverrideDefault("System.getProperty(\"java.io.tmpdir\")") public static final ConfigOption<String> TMP_DIR = key("web.tmpdir") .defaultValue(System.getProperty("java.io.tmpdir")) http://git-wip-us.apache.org/repos/asf/flink/blob/c7d59105/flink-docs/src/main/java/org/apache/flink/docs/configuration/ConfigOptionsDocGenerator.java ---------------------------------------------------------------------- diff --git a/flink-docs/src/main/java/org/apache/flink/docs/configuration/ConfigOptionsDocGenerator.java b/flink-docs/src/main/java/org/apache/flink/docs/configuration/ConfigOptionsDocGenerator.java index b51a028..aadeee6 100644 --- a/flink-docs/src/main/java/org/apache/flink/docs/configuration/ConfigOptionsDocGenerator.java +++ b/flink-docs/src/main/java/org/apache/flink/docs/configuration/ConfigOptionsDocGenerator.java @@ -21,10 +21,9 @@ package org.apache.flink.docs.configuration; import org.apache.flink.annotation.VisibleForTesting; import org.apache.flink.annotation.docs.ConfigGroup; import org.apache.flink.annotation.docs.ConfigGroups; +import org.apache.flink.annotation.docs.Documentation; import org.apache.flink.api.java.tuple.Tuple2; import org.apache.flink.configuration.ConfigOption; -import org.apache.flink.configuration.CoreOptions; -import org.apache.flink.configuration.WebOptions; import org.apache.flink.util.function.ThrowingConsumer; import java.io.IOException; @@ -126,7 +125,7 @@ public class ConfigOptionsDocGenerator { @VisibleForTesting static List<Tuple2<ConfigGroup, String>> generateTablesForClass(Class<?> optionsClass) { ConfigGroups configGroups = optionsClass.getAnnotation(ConfigGroups.class); - List<ConfigOption<?>> allOptions = extractConfigOptions(optionsClass); + List<OptionWithMetaInfo> allOptions = extractConfigOptions(optionsClass); List<Tuple2<ConfigGroup, String>> tables; if (configGroups != null) { @@ -134,11 +133,11 @@ public class ConfigOptionsDocGenerator { Tree tree = new Tree(configGroups.groups(), allOptions); for (ConfigGroup group : configGroups.groups()) { - List<ConfigOption<?>> configOptions = tree.findConfigOptions(group); + List<OptionWithMetaInfo> configOptions = tree.findConfigOptions(group); sortOptions(configOptions); tables.add(Tuple2.of(group, toHtmlTable(configOptions))); } - List<ConfigOption<?>> configOptions = tree.getDefaultOptions(); + List<OptionWithMetaInfo> configOptions = tree.getDefaultOptions(); sortOptions(configOptions); tables.add(Tuple2.of(null, toHtmlTable(configOptions))); } else { @@ -148,13 +147,13 @@ public class ConfigOptionsDocGenerator { return tables; } - private static List<ConfigOption<?>> extractConfigOptions(Class<?> clazz) { + private static List<OptionWithMetaInfo> extractConfigOptions(Class<?> clazz) { try { - List<ConfigOption<?>> configOptions = new ArrayList<>(8); + List<OptionWithMetaInfo> configOptions = new ArrayList<>(8); Field[] fields = clazz.getFields(); for (Field field : fields) { if (field.getType().equals(ConfigOption.class) && field.getAnnotation(Deprecated.class) == null) { - configOptions.add((ConfigOption<?>) field.get(null)); + configOptions.add(new OptionWithMetaInfo((ConfigOption<?>) field.get(null), field)); } } @@ -171,7 +170,7 @@ public class ConfigOptionsDocGenerator { * @param options list of options to include in this group * @return string containing HTML formatted table */ - private static String toHtmlTable(final List<ConfigOption<?>> options) { + private static String toHtmlTable(final List<OptionWithMetaInfo> options) { StringBuilder htmlTable = new StringBuilder(); htmlTable.append("<table class=\"table table-bordered\">\n"); htmlTable.append(" <thead>\n"); @@ -183,7 +182,7 @@ public class ConfigOptionsDocGenerator { htmlTable.append(" </thead>\n"); htmlTable.append(" <tbody>\n"); - for (ConfigOption<?> option : options) { + for (OptionWithMetaInfo option : options) { htmlTable.append(toHtmlString(option)); } @@ -196,21 +195,23 @@ public class ConfigOptionsDocGenerator { /** * Transforms option to table row. * - * @param option option to transform + * @param optionWithMetaInfo option to transform * @return row with the option description */ - private static String toHtmlString(final ConfigOption<?> option) { - Object defaultValue = option.defaultValue(); - // This is a temporary hack that should be removed once FLINK-6490 is resolved. - // These options use System.getProperty("java.io.tmpdir") as the default. - // As a result the generated table contains an actual path as the default, which is simply wrong. - if (option == WebOptions.TMP_DIR || option.key().equals("python.dc.tmp.dir") || option == CoreOptions.TMP_DIRS) { - defaultValue = null; + private static String toHtmlString(final OptionWithMetaInfo optionWithMetaInfo) { + ConfigOption<?> option = optionWithMetaInfo.option; + String defaultValue; + + Documentation.OverrideDefault overrideDocumentedDefault = optionWithMetaInfo.field.getAnnotation(Documentation.OverrideDefault.class); + if (overrideDocumentedDefault != null) { + defaultValue = escapeCharacters(addWordBreakOpportunities(overrideDocumentedDefault.value())); + } else { + defaultValue = escapeCharacters(addWordBreakOpportunities(defaultValueToHtml(option.defaultValue()))); } return "" + " <tr>\n" + " <td><h5>" + escapeCharacters(option.key()) + "</h5></td>\n" + - " <td style=\"word-wrap: break-word;\">" + escapeCharacters(addWordBreakOpportunities(defaultValueToHtml(defaultValue))) + "</td>\n" + + " <td style=\"word-wrap: break-word;\">" + defaultValue + "</td>\n" + " <td>" + escapeCharacters(option.description()) + "</td>\n" + " </tr>\n"; } @@ -240,8 +241,8 @@ public class ConfigOptionsDocGenerator { .replace(";", ";<wbr>"); } - private static void sortOptions(List<ConfigOption<?>> configOptions) { - configOptions.sort(Comparator.comparing(ConfigOption::key)); + private static void sortOptions(List<OptionWithMetaInfo> configOptions) { + configOptions.sort(Comparator.comparing(option -> option.option.key())); } /** @@ -251,7 +252,7 @@ public class ConfigOptionsDocGenerator { private static class Tree { private final Node root = new Node(); - Tree(ConfigGroup[] groups, Collection<ConfigOption<?>> options) { + Tree(ConfigGroup[] groups, Collection<OptionWithMetaInfo> options) { // generate a tree based on all key prefixes for (ConfigGroup group : groups) { String[] keyComponents = group.keyPrefix().split("\\."); @@ -264,17 +265,17 @@ public class ConfigOptionsDocGenerator { // assign options to their corresponding group, i.e. the last group root node encountered when traversing // the tree based on the option key - for (ConfigOption<?> option : options) { - findGroupRoot(option.key()).assignOption(option); + for (OptionWithMetaInfo option : options) { + findGroupRoot(option.option.key()).assignOption(option); } } - List<ConfigOption<?>> findConfigOptions(ConfigGroup configGroup) { + List<OptionWithMetaInfo> findConfigOptions(ConfigGroup configGroup) { Node groupRoot = findGroupRoot(configGroup.keyPrefix()); return groupRoot.getConfigOptions(); } - List<ConfigOption<?>> getDefaultOptions() { + List<OptionWithMetaInfo> getDefaultOptions() { return root.getConfigOptions(); } @@ -288,7 +289,7 @@ public class ConfigOptionsDocGenerator { } private static class Node { - private final List<ConfigOption<?>> configOptions = new ArrayList<>(8); + private final List<OptionWithMetaInfo> configOptions = new ArrayList<>(8); private final Map<String, Node> children = new HashMap<>(8); private boolean isGroupRoot = false; @@ -309,7 +310,7 @@ public class ConfigOptionsDocGenerator { return child; } - private void assignOption(ConfigOption<?> option) { + private void assignOption(OptionWithMetaInfo option) { configOptions.add(option); } @@ -321,12 +322,22 @@ public class ConfigOptionsDocGenerator { this.isGroupRoot = true; } - private List<ConfigOption<?>> getConfigOptions() { + private List<OptionWithMetaInfo> getConfigOptions() { return configOptions; } } } + private static class OptionWithMetaInfo { + private final ConfigOption<?> option; + private final Field field; + + public OptionWithMetaInfo(ConfigOption<?> option, Field field) { + this.option = option; + this.field = field; + } + } + private ConfigOptionsDocGenerator() { } } http://git-wip-us.apache.org/repos/asf/flink/blob/c7d59105/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocGeneratorTest.java ---------------------------------------------------------------------- diff --git a/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocGeneratorTest.java b/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocGeneratorTest.java index ba30617..4176c5d 100644 --- a/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocGeneratorTest.java +++ b/flink-docs/src/test/java/org/apache/flink/docs/configuration/ConfigOptionsDocGeneratorTest.java @@ -20,6 +20,7 @@ package org.apache.flink.docs.configuration; import org.apache.flink.annotation.docs.ConfigGroup; import org.apache.flink.annotation.docs.ConfigGroups; +import org.apache.flink.annotation.docs.Documentation; import org.apache.flink.api.java.tuple.Tuple2; import org.apache.flink.configuration.ConfigOption; import org.apache.flink.configuration.ConfigOptions; @@ -171,4 +172,47 @@ public class ConfigOptionsDocGeneratorTest { "</table>\n", tablesConverted.get("default")); } + static class TestConfigGroupWithOverriddenDefault { + @Documentation.OverrideDefault("default_1") + public static ConfigOption<Integer> firstOption = ConfigOptions + .key("first.option.a") + .defaultValue(2) + .withDescription("This is example description for the first option."); + + @Documentation.OverrideDefault("default_2") + public static ConfigOption<String> secondOption = ConfigOptions + .key("second.option.a") + .noDefaultValue() + .withDescription("This is long example description for the second option."); + } + + @Test + public void testOverrideDefault() { + final String expectedTable = + "<table class=\"table table-bordered\">\n" + + " <thead>\n" + + " <tr>\n" + + " <th class=\"text-left\" style=\"width: 20%\">Key</th>\n" + + " <th class=\"text-left\" style=\"width: 15%\">Default</th>\n" + + " <th class=\"text-left\" style=\"width: 65%\">Description</th>\n" + + " </tr>\n" + + " </thead>\n" + + " <tbody>\n" + + " <tr>\n" + + " <td><h5>first.option.a</h5></td>\n" + + " <td style=\"word-wrap: break-word;\">default_1</td>\n" + + " <td>This is example description for the first option.</td>\n" + + " </tr>\n" + + " <tr>\n" + + " <td><h5>second.option.a</h5></td>\n" + + " <td style=\"word-wrap: break-word;\">default_2</td>\n" + + " <td>This is long example description for the second option.</td>\n" + + " </tr>\n" + + " </tbody>\n" + + "</table>\n"; + final String htmlTable = ConfigOptionsDocGenerator.generateTablesForClass(TestConfigGroupWithOverriddenDefault.class).get(0).f1; + + assertEquals(expectedTable, htmlTable); + } + }
