zentol commented on a change in pull request #8445: 
[FLINK-12127][network,config] Move network related options form 
TaskManagerOptions and NettyConfig into NetworkEnvironmentOptions
URL: https://github.com/apache/flink/pull/8445#discussion_r285097169
 
 

 ##########
 File path: 
flink-runtime/src/main/java/org/apache/flink/runtime/io/network/netty/NettyConfig.java
 ##########
 @@ -38,53 +36,6 @@
 
        private static final Logger LOG = 
LoggerFactory.getLogger(NettyConfig.class);
 
-       // - Config keys 
----------------------------------------------------------
-
-       public static final ConfigOption<Integer> NUM_ARENAS = ConfigOptions
 
 Review comment:
   I'm not sure what the benefit of moving these is. The `NetworkEnvironment` 
_itself_ isn't even using them (and I supposed never will since they are 
details of the netty implementation).
   I'd keep them here to prevent `NetworkEnvironmentOptions` from becoming a 
hodge-podge of options for various shuffle-service implementations as well as 
generic ones.
   
   This is in a way quite similar to how we handle YARN/Mesos/etc.; the 
`ResourceManagerOptions` contain generic options for all container 
environments, but we still have distinct `*Options` classes for each.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to