[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-03-22 Thread dawidwys
Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3495 Could someone have a look on it? I saw other modules start to migrate to `ConfigOption` so I think would be nice to use it for docs generation. --- If your project is set up for it, you can reply t

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-08 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 I'll check this out again today. I will try to integrate the generated tables into the respective documentation to see if there isn't some edge case we haven't considered. If I don't find any

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-08 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 Ran into an issue. Some config parameters, like `jobmanager.web.tmpdir`, have non-constant default values that are evaluated at runtime, like `System.getProperty("java.tmpdir")`. These values now show

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-08 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 These options are affected by the above: ```JobManagerOptions#WEB_TMP_DIR``` ```PythonOptions#DC_TMP_DIR``` The ```HistoryServerOptions#HISTORY_WEB_DIR``` does not define a default val

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-08 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 I'm also thinking that we should hide ConfigOptions that are marked as deprecated, which should be rather easy to fix since we're already employing reflection. --- If your project is set up for it,

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-09 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 2 more issues popped up: A large number of configuration parameters have not been ported yet (see [FLINK-4765](https://issues.apache.org/jira/browse/FLINK-4765) which means that we cannot use

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-10 Thread dawidwys
Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3495 There is a problem with the prefix solution. See the mentioned JobManager and WebFrontentd options, prefix for JobManager options is "jobmanager" which also covers prefix for WebFrontend options whi

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-10 Thread dawidwys
Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3495 How about adding another class like ConfigGroup? ``` public static final ConfigGroup WEB_FRONTEND = new ConfigGroup("WebFrontend", WEB_BACKPRESSURE_DELAY, WEB_BACKPRESSURE_NUM

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-10 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 If we allow supplying a regex instead we could separate "jobmanager" and "jobmanager.web". Or we could could make the matching exclusive and start with the most specific groups first. I think we can s

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-10 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 I love this issue, it's just so damn interesting. You can find an implementation of my approach [here](https://github.com/zentol/flink/commit/2c40e660a883ec4db28cf92913daf0a103563fc8).

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-11 Thread dawidwys
Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3495 I do like you code, only thing missing I had in mind yesterday was an idea of defaultGroup, which is an easy way to group options with not overlapping prefixes e.g. right now in documentation (in do

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-11 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 Couldn't we create the default group implicitly by taking all options that aren't part of any ConfigGroup? As in having ConfigGroup only describing sub-groups that should be excluded from the default

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-11 Thread dawidwys
Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3495 It works exactly as you describe. The role of `defaultGroup` in `ConfigGroups` is setting a name of the defaultGroup. I agree using `ConfigGroup` for the `defaultGroup` may be counter intuitive.

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-11 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 I would use the class name, as otherwise we add an additional source of mistakes. When introducing the `ConfigGroups` annotation the first time; if the configured default name doesn't match the class

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-15 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 I think we're good to go now; will try it out again after a rebase and merge it unless something major comes up. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-15 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 i will make a number of changes before merging it. - added a hack to deal with options using `System.getProperty("java.io.tmpdir")` as the default value - replace `>` and `<` in descriptio

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-04-06 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 I'm playing around with this right now. Seems like it works; don't know though if the resulting html is what people expected; since you know, it is pretty bare-bones. :) Anyway, I'm looking i

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-04-06 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 Alright here's what i came up with. We define a single antrun task which does the following: Since all *Options classes reside in the same directory we just take a look in there and filter ou

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-04-06 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 This feature only really makes sense if we can integrate it directly into the web documentation. This should actually be rather simple easy. 1) Create a copy of the existing `/docs/setup/c

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-04-07 Thread dawidwys
Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3495 Ad second comment: My intention was to include the generated tables into markdown using command like: `{% include generated/high_availability_configuration.html %}` This way it is possib

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-04-07 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 That's a neat idea to re-use them. Let's make this part of the PR though. You aren't limited to flink-core, you just have to put the plugin into other modules as well. You would have to to the

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-04-23 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 This is working pretty nicely. Just tried out appending the generated table to the SSL documentation, here's the result: ![ssl_example](https://cloud.githubusercontent.com/assets/5725237/25316434/

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-04-23 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 I'm also not sure about including the short description. Looking at the existing documentation descriptions this would result in pretty large table entries with a lot of whitespace in the short descri

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-04-24 Thread dawidwys
Github user dawidwys commented on the issue: https://github.com/apache/flink/pull/3495 I will address the issues with formatting default value tomorrow morning. As for the `shortDescription` it came out on the initiative of ML folks (at least it is written as such in [FLINK-5

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-04-24 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 Wouldn't be surprised if @uce could provide some useful input regarding the short description. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub a

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-03 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 We should also update the docs for building the docs (`docs/README.md`) with instructions on how to regenerate the tables. A small note on how to integrate these tables into the docs would be sweet to

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-03 Thread zentol
Github user zentol commented on the issue: https://github.com/apache/flink/pull/3495 Also, let's drop the short descriptions. If we ever feel the need to have them we can always add them again. --- If your project is set up for it, you can reply to this email and have your reply appe

[GitHub] flink issue #3495: [FLINK-5781][config] Generation HTML from ConfigOption

2017-05-03 Thread uce
Github user uce commented on the issue: https://github.com/apache/flink/pull/3495 I think it's up to you guys whether you want the short description in there or not. I see that it only makes sense if you actually invest the time to have a good short description. You would probably nee