Skip to content

Commit

Permalink
[FLINK-15824][docs] (follow-up) Simple improvements/cleanups on @Sect…
Browse files Browse the repository at this point in the history
…ionOption

  - Rename '@SectionOption' to '@section' for brevity

  - Rename '@Section.sections()' to '@Section.value()'
    That way, the method name can be skipped when only sections (no positions) are specified,
    which should is the common case after config documentation rework is complete.

  - Adjust test for @section annotation
  • Loading branch information
StephanEwen committed Feb 3, 2020
1 parent 2ce0409 commit c8ade87
Show file tree
Hide file tree
Showing 11 changed files with 161 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,13 @@ public final class Documentation {
* Annotation used on config option fields to include them in specific sections. Sections are groups of options
* that are aggregated across option classes, with each group being placed into a dedicated file.
*
* <p>The {@link SectionOption#position()} argument controls the position in the generated table, with lower values
* <p>The {@link Section#position()} argument controls the position in the generated table, with lower values
* being placed at the top. Fields with the same position are sorted alphabetically by key.
*/
@Target(ElementType.FIELD)
@Retention(RetentionPolicy.RUNTIME)
@Internal
public @interface SectionOption {
public @interface Section {
int POSITION_MEMORY = 10;
int POSITION_PARALLELISM_SLOTS = 20;
int POSITION_FAULT_TOLERANCE = 30;
Expand All @@ -62,7 +62,7 @@ public final class Documentation {
/**
* The sections in the config docs where this option should be included.
*/
String[] sections() default {};
String[] value() default {};

/**
* The relative position of the option in its section.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ public class CheckpointingOptions {
// ------------------------------------------------------------------------

/** The state backend to be used to store and checkpoint state. */
@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_FAULT_TOLERANCE)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_FAULT_TOLERANCE)
public static final ConfigOption<String> STATE_BACKEND = ConfigOptions
.key("state.backend")
.noDefaultValue()
Expand Down Expand Up @@ -105,9 +105,9 @@ public class CheckpointingOptions {

/** The default directory for savepoints. Used by the state backends that write
* savepoints to file systems (MemoryStateBackend, FsStateBackend, RocksDBStateBackend). */
@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_FAULT_TOLERANCE)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_FAULT_TOLERANCE)
public static final ConfigOption<String> SAVEPOINT_DIRECTORY = ConfigOptions
.key("state.savepoints.dir")
.noDefaultValue()
Expand All @@ -117,9 +117,9 @@ public class CheckpointingOptions {

/** The default directory used for storing the data files and meta data of checkpoints in a Flink supported filesystem.
* The storage path must be accessible from all participating processes/nodes(i.e. all TaskManagers and JobManagers).*/
@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_FAULT_TOLERANCE)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_FAULT_TOLERANCE)
public static final ConfigOption<String> CHECKPOINTS_DIRECTORY = ConfigOptions
.key("state.checkpoints.dir")
.noDefaultValue()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,9 @@ private static String[] parseParentFirstLoaderPatterns(String base, String appen
// program
// ------------------------------------------------------------------------

@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_PARALLELISM_SLOTS)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_PARALLELISM_SLOTS)
public static final ConfigOption<Integer> DEFAULT_PARALLELISM = ConfigOptions
.key("parallelism.default")
.defaultValue(1)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ public class HighAvailabilityOptions {
* To enable high-availability, set this mode to "ZOOKEEPER".
* Can also be set to FQN of HighAvailability factory class.
*/
@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_HIGH_AVAILABILITY)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_HIGH_AVAILABILITY)
public static final ConfigOption<String> HA_MODE =
key("high-availability")
.defaultValue("NONE")
Expand All @@ -69,9 +69,9 @@ public class HighAvailabilityOptions {
/**
* File system path (URI) where Flink persists metadata in high-availability setups.
*/
@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_HIGH_AVAILABILITY)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_HIGH_AVAILABILITY)
public static final ConfigOption<String> HA_STORAGE_PATH =
key("high-availability.storageDir")
.noDefaultValue()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,9 @@ public class JobManagerOptions {
/**
* JVM heap size for the JobManager with memory size.
*/
@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_MEMORY)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_MEMORY)
public static final ConfigOption<String> JOB_MANAGER_HEAP_MEMORY =
key("jobmanager.heap.size")
.defaultValue("1024m")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,9 @@ public class SecurityOptions {
/**
* Enable SSL for internal communication (akka rpc, netty data transport, blob server).
*/
@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_SECURITY)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_SECURITY)
public static final ConfigOption<Boolean> SSL_INTERNAL_ENABLED =
key("security.ssl.internal.enabled")
.defaultValue(false)
Expand All @@ -119,9 +119,9 @@ public class SecurityOptions {
/**
* Enable SSL for external REST endpoints.
*/
@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_SECURITY)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_SECURITY)
public static final ConfigOption<Boolean> SSL_REST_ENABLED =
key("security.ssl.rest.enabled")
.defaultValue(false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,9 @@ public class TaskManagerOptions {
/**
* The config parameter defining the number of task slots of a task manager.
*/
@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_PARALLELISM_SLOTS)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_PARALLELISM_SLOTS)
public static final ConfigOption<Integer> NUM_TASK_SLOTS =
key("taskmanager.numberOfTaskSlots")
.intType()
Expand Down Expand Up @@ -254,9 +254,9 @@ public class TaskManagerOptions {
/**
* Total Process Memory size for the TaskExecutors.
*/
@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_MEMORY)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_MEMORY)
public static final ConfigOption<MemorySize> TOTAL_PROCESS_MEMORY =
key("taskmanager.memory.process.size")
.memoryType()
Expand All @@ -270,9 +270,9 @@ public class TaskManagerOptions {
/**
* Total Flink Memory size for the TaskExecutors.
*/
@Documentation.SectionOption(
sections = {Documentation.SectionOption.SECTION_COMMON},
position = Documentation.SectionOption.POSITION_MEMORY)
@Documentation.Section(
value = {Documentation.Section.SECTION_COMMON},
position = Documentation.Section.POSITION_MEMORY)
public static final ConfigOption<MemorySize> TOTAL_FLINK_MEMORY =
key("taskmanager.memory.flink.size")
.memoryType()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ public class ConfigOptionsDocGenerator {
* every {@link ConfigOption}.
*
* <p>One additional table is generated containing all {@link ConfigOption ConfigOptions} that are annotated with
* {@link Documentation.SectionOption}.
* {@link Documentation.Section}.
*
* @param args
* [0] output directory for the generated files
Expand All @@ -127,12 +127,12 @@ static void generateCommonSection(String rootDir, String outputDirectory, Option

Map<String, List<OptionWithMetaInfo>> optionsGroupedBySection = allSectionOptions.stream()
.flatMap(option -> {
final String[] sections = option.field.getAnnotation(Documentation.SectionOption.class).sections();
final String[] sections = option.field.getAnnotation(Documentation.Section.class).value();
if (sections.length == 0) {
throw new RuntimeException(String.format(
"Option %s is annotated with %s but the list of sections is empty.",
option.option.key(),
Documentation.SectionOption.class.getSimpleName()));
Documentation.Section.class.getSimpleName()));
}

return Arrays.stream(sections).map(section -> Tuple2.of(section, option));
Expand All @@ -142,8 +142,8 @@ static void generateCommonSection(String rootDir, String outputDirectory, Option
optionsGroupedBySection.forEach(
(section, options) -> {
options.sort((o1, o2) -> {
int position1 = o1.field.getAnnotation(Documentation.SectionOption.class).position();
int position2 = o2.field.getAnnotation(Documentation.SectionOption.class).position();
int position1 = o1.field.getAnnotation(Documentation.Section.class).position();
int position2 = o2.field.getAnnotation(Documentation.Section.class).position();
if (position1 == position2) {
return o1.option.key().compareTo(o2.option.key());
} else {
Expand All @@ -169,7 +169,7 @@ static String getSectionFileName(String section) {
private static Collection<OptionWithMetaInfo> findSectionOptions(String rootDir, String module, String packageName, String pathPrefix) throws IOException, ClassNotFoundException {
Collection<OptionWithMetaInfo> commonOptions = new ArrayList<>(32);
processConfigOptions(rootDir, module, packageName, pathPrefix, optionsClass -> extractConfigOptions(optionsClass).stream()
.filter(optionWithMetaInfo -> optionWithMetaInfo.field.getAnnotation(Documentation.SectionOption.class) != null)
.filter(optionWithMetaInfo -> optionWithMetaInfo.field.getAnnotation(Documentation.Section.class) != null)
.forEachOrdered(commonOptions::add));
return commonOptions;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
import org.junit.Test;
import org.junit.rules.TemporaryFolder;

import java.io.File;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Paths;
import java.time.Duration;
import java.util.Collections;
import java.util.HashMap;
Expand Down Expand Up @@ -423,8 +423,8 @@ public void testOverrideDefault() {
}

@Test
public void testCommonOptions() throws IOException, ClassNotFoundException {
final String projectRootDir = System.getProperty("rootDir");
public void testSections() throws IOException, ClassNotFoundException {
final String projectRootDir = getProjectRootDir();
final String outputDirectory = TMP.newFolder().getAbsolutePath();

final OptionsClassLocation[] locations = new OptionsClassLocation[] {
Expand All @@ -434,7 +434,7 @@ public void testCommonOptions() throws IOException, ClassNotFoundException {
ConfigOptionsDocGenerator.generateCommonSection(projectRootDir, outputDirectory, locations, "src/test/java");
Formatter formatter = new HtmlFormatter();

String expected =
String expected1 =
"<table class=\"table table-bordered\">\n" +
" <thead>\n" +
" <tr>\n" +
Expand All @@ -460,9 +460,34 @@ public void testCommonOptions() throws IOException, ClassNotFoundException {
" </tbody>\n" +
"</table>\n";

String output = FileUtils.readFile(Paths.get(outputDirectory, ConfigOptionsDocGenerator.getSectionFileName("common")).toFile(), StandardCharsets.UTF_8.name());
String expected2 =
"<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: 10%\">Type</th>\n" +
" <th class=\"text-left\" style=\"width: 55%\">Description</th>\n" +
" </tr>\n" +
" </thead>\n" +
" <tbody>\n" +
" <tr>\n" +
" <td><h5>" + TestCommonOptions.COMMON_OPTION.key() + "</h5></td>\n" +
" <td style=\"word-wrap: break-word;\">" + TestCommonOptions.COMMON_OPTION.defaultValue() + "</td>\n" +
" <td>Integer</td>\n" +
" <td>" + formatter.format(TestCommonOptions.COMMON_OPTION.description()) + "</td>\n" +
" </tr>\n" +
" </tbody>\n" +
"</table>\n";

final String fileName1 = ConfigOptionsDocGenerator.getSectionFileName(TestCommonOptions.SECTION_1);
final String fileName2 = ConfigOptionsDocGenerator.getSectionFileName(TestCommonOptions.SECTION_2);

assertEquals(expected, output);
final String output1 = FileUtils.readFile(new File(outputDirectory, fileName1), StandardCharsets.UTF_8.name());
final String output2 = FileUtils.readFile(new File(outputDirectory, fileName2), StandardCharsets.UTF_8.name());

assertEquals(expected1, output1);
assertEquals(expected2, output2);
}

static class TestConfigGroupWithExclusion {
Expand Down Expand Up @@ -509,4 +534,15 @@ public void testConfigOptionExclusion() {

assertEquals(expectedTable, htmlTable);
}

static String getProjectRootDir() {
final String dirFromProperty = System.getProperty("rootDir");
if (dirFromProperty != null) {
return dirFromProperty;
}

// to make this work in the IDE use a default fallback
final String currentDir = System.getProperty("user.dir");
return new File(currentDir).getParent();
}
}
Loading

0 comments on commit c8ade87

Please sign in to comment.