Skip to content

Conversation

zentol
Copy link
Contributor

@zentol zentol commented May 19, 2017

This is a follow-up to c102547; I missed some usages of ConfigConstants#JOB_MANAGER_WEB_PORT_KEY.

*/
val webMonitorPort : Int = flinkConfiguration.getInteger(
ConfigConstants.JOB_MANAGER_WEB_PORT_KEY, -1)
JobManagerOptions.WEB_PORT.key(), -1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure, but why don't you use flinkConfiguration.getInteger(JobManagerOptions.WEB_PORT)? It would have different semantics (it would return 8081 instead of -1 in case of missing value).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about doing that, but decided against it to preserve existing behavior. If no port is configured at all, with the current code, a random port will be used. Someone out there may be relying on this (although they should just explicitly set it to 0)

@zentol
Copy link
Contributor Author

zentol commented Jun 26, 2017

merging.

zentol added a commit to zentol/flink that referenced this pull request Jun 26, 2017
zentol added a commit to zentol/flink that referenced this pull request Jun 27, 2017
zentol added a commit to zentol/flink that referenced this pull request Jun 27, 2017
zentol added a commit to zentol/flink that referenced this pull request Jun 28, 2017
zentol added a commit to zentol/flink that referenced this pull request Jun 28, 2017
zentol added a commit to zentol/flink that referenced this pull request Jun 28, 2017
zentol added a commit to zentol/flink that referenced this pull request Jun 29, 2017
zentol added a commit to zentol/flink that referenced this pull request Jun 30, 2017
@asfgit asfgit closed this in 80b2805 Jul 1, 2017
@zentol zentol deleted the 6461_followup branch July 1, 2017 12:24
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.

3 participants