Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLINK-8744][docs] Generate "Common Option" section #5843

Closed
wants to merge 6 commits into from

Conversation

zentol
Copy link
Contributor

@zentol zentol commented Apr 12, 2018

What is the purpose of the change

With this PR the "Common Options" section is now generated by the ConfigOptionDocsGenerator.

Brief change log

  • add ´CommonOption` annotation
  • annotate a few options with added annotation
    • I was very conservative as i do not want to discuss in this PR which options should be regarded as common
  • allow passing in a path prefix for testing purposes; this allows testing against Options* classes residing in the test scope
  • extend the generator to gather all annotated options and generated a HTML table
  • replace common section with generated file
  • extend completeness test to also cover common section
  • remove advanced section, as subsumed by full reference

Verifying this change

  • run ConfigOptionsDocGeneratorTest#testCommonOptions
  • run ConfigOptionsDocsCompletenessTest#testCommonSectionCompleteness

@StephanEwen
Copy link
Contributor

Looks pretty good.

Is there a way we can "sort" the common options? Something like

  • host:port (for standalone setups)
  • java memory
  • default parallelism / slots
  • fault tolerance
  • HA
  • security

@zentol
Copy link
Contributor Author

zentol commented May 24, 2018

they should be sorted alphabetically based on the key, but i now see that this isn't actually the case.

I'm not sure if I can easily add the kind of sorting you're asking for as that would require the generator to know about the semantics of an option ("oh, this is a port option, but that one is about memory!").

@StephanEwen
Copy link
Contributor

One could add an int to the annotation, as "priority / position" and sort by that.
Not sure nice, but could be okay.

I think it was nice for users that the most common options (the ones you need first) were at the top of the list.

Out of curiosity, what happens to options like env.java.opts which are shell script only options, but very common?

@zentol
Copy link
Contributor Author

zentol commented May 24, 2018

For env.java.opts we have a ConfigOption in the CoreOptions class. They are documented like any other option (and technically they are also usable like any other option).

@zentol
Copy link
Contributor Author

zentol commented May 28, 2018

@StephanEwen The options are now sorted.

@StefanRRichter
Copy link
Contributor

LGTM 👍

@zentol
Copy link
Contributor Author

zentol commented Jun 14, 2018

merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants