Skip to content

Commit

Permalink
[FLINK-8743][docs] Allow overriding documented default
Browse files Browse the repository at this point in the history
This closes apache#5822.
  • Loading branch information
zentol committed May 2, 2018
1 parent 678ab6c commit c7d5910
Show file tree
Hide file tree
Showing 7 changed files with 135 additions and 31 deletions.
2 changes: 1 addition & 1 deletion docs/_includes/generated/core_configuration.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
2 changes: 1 addition & 1 deletion docs/_includes/generated/web_configuration.html
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
@@ -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
*
* https://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(){
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
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;

Expand Down Expand Up @@ -180,6 +181,7 @@ public static String[] getParentFirstLoaderPatterns(Configuration config) {
* 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"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,9 @@
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;
Expand Down Expand Up @@ -126,19 +125,19 @@ private static void processConfigOptions(String rootDir, String module, String p
@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) {
tables = new ArrayList<>(configGroups.groups().length + 1);
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 {
Expand All @@ -148,13 +147,13 @@ static List<Tuple2<ConfigGroup, String>> generateTablesForClass(Class<?> options
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));
}
}

Expand All @@ -171,7 +170,7 @@ private static List<ConfigOption<?>> extractConfigOptions(Class<?> clazz) {
* @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");
Expand All @@ -183,7 +182,7 @@ private static String toHtmlTable(final List<ConfigOption<?>> options) {
htmlTable.append(" </thead>\n");
htmlTable.append(" <tbody>\n");

for (ConfigOption<?> option : options) {
for (OptionWithMetaInfo option : options) {
htmlTable.append(toHtmlString(option));
}

Expand All @@ -196,21 +195,23 @@ private static String toHtmlTable(final List<ConfigOption<?>> options) {
/**
* 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";
}
Expand Down Expand Up @@ -240,8 +241,8 @@ private static String addWordBreakOpportunities(String value) {
.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()));
}

/**
Expand All @@ -251,7 +252,7 @@ private static void sortOptions(List<ConfigOption<?>> configOptions) {
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("\\.");
Expand All @@ -264,17 +265,17 @@ private static class Tree {

// 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();
}

Expand All @@ -288,7 +289,7 @@ private Node findGroupRoot(String key) {
}

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;

Expand All @@ -309,7 +310,7 @@ private Node findChild(String keyComponent) {
return child;
}

private void assignOption(ConfigOption<?> option) {
private void assignOption(OptionWithMetaInfo option) {
configOptions.add(option);
}

Expand All @@ -321,12 +322,22 @@ private void markAsGroupRoot() {
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() {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

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;
Expand Down Expand Up @@ -171,4 +172,47 @@ public void testCreatingMultipleGroups() {
"</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);
}

}

0 comments on commit c7d5910

Please sign in to comment.