[GitHub] zentol commented on issue #6872: [FLINK-10436] Add ConfigOption#withFallbackKeys
zentol commented on issue #6872: [FLINK-10436] Add ConfigOption#withFallbackKeys URL: https://github.com/apache/flink/pull/6872#issuecomment-431310391 @tillrohrmann agreed, this isn't strictly about the PR. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zentol commented on issue #6872: [FLINK-10436] Add ConfigOption#withFallbackKeys
zentol commented on issue #6872: [FLINK-10436] Add ConfigOption#withFallbackKeys URL: https://github.com/apache/flink/pull/6872#issuecomment-431256748 > If we now introduce a RestOption#DISPATCHER_ADDRESS with the fallback to RestOption#ADDRESS and JobManagerOptions#ADDRESS and remove the latter as a deprecated key from the former, then we break setups. > In the future, this might change and could require to remove the fallback dependency depending on what the default behaviour is. This also applies to your proposed solution for handling this issue in the future. Although ... is this really is an issue? We can change all usages within Flink to `DISPATCHER_ADDRESS` (which should be reasonable considering that we only ever go to the dispatcher via REST), with the exception of the `RestServerEndpoint` for which we map ' DISPATCHER_ADDRESS' to ' REST#ADDRESS'. What could this break? I can only come up with a custom `ClusterEntryPoint` that uses a `RestServerEndpoint` internally and no other Flink component that is aware of the configuration thingie, which seems unlikely? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zentol commented on issue #6872: [FLINK-10436] Add ConfigOption#withFallbackKeys
zentol commented on issue #6872: [FLINK-10436] Add ConfigOption#withFallbackKeys URL: https://github.com/apache/flink/pull/6872#issuecomment-431087755 I understood what the fallback is supposed to do, but it doesn't make sense that something generic like `rest.address` uses the very specific `jobmanager.rpc.address` as a fall-back. Falling back to a _more specific_ option is conceptually not sound. If you want to make the fallback behavior re-usable I'd suggest a dedicated `dispatcher.rest.port` option with fall-backs to `rest.port` and `jobmanager.rpc.address` which you then use to configure the options that the `RestEndpoint` expects. This way you retain the convenient fallback behavior without coupling the rest configuration to any other component. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zentol commented on issue #6872: [FLINK-10436] Add ConfigOption#withFallbackKeys
zentol commented on issue #6872: [FLINK-10436] Add ConfigOption#withFallbackKeys URL: https://github.com/apache/flink/pull/6872#issuecomment-430948817 I'm aware in my example the same issue applies to a deprecated key, but that suggests to me that the deprecated key shouldn't exist and the component should deal with this case explicitly. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] zentol commented on issue #6872: [FLINK-10436] Add ConfigOption#withFallbackKeys
zentol commented on issue #6872: [FLINK-10436] Add ConfigOption#withFallbackKeys URL: https://github.com/apache/flink/pull/6872#issuecomment-430948482 Aren't fallback-keys really dependent on the context in which the option is used? In the case of `rest.port` a fallback to `jobmanager.web.port` only makes sense when you're on the dispatcher; if any other component wants to use `rest.port` as well they are now also using dispatcher configuration values. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
