You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by GitBox <gi...@apache.org> on 2020/06/25 17:58:17 UTC
[GitHub] [storm] Ethanlm commented on a change in pull request #3293: [STORM-3657] set storm.messaging.netty.authentication to topoConf OR daemonConf
Ethanlm commented on a change in pull request #3293:
URL: https://github.com/apache/storm/pull/3293#discussion_r445732035
##########
File path: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
##########
@@ -3165,6 +3163,16 @@ public void submitTopologyWithOpts(String topoName, String uploadedJarLocation,
topoConf.remove(Config.TOPOLOGY_CLASSPATH_BEGINNING);
}
+ // storm.messaging.netty.authentication is about inter-worker communication
+ // enforce netty authentication when either topo or daemon set it to true
+ boolean enforceNettyAuth = (Boolean) topoConf.getOrDefault(Config.STORM_MESSAGING_NETTY_AUTHENTICATION, false)
Review comment:
`storm.messaging.netty.authentication` is guaranteed to be set because it is in `defaults.yaml`. We don't want to have default value in different places.
##########
File path: storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java
##########
@@ -3165,6 +3163,16 @@ public void submitTopologyWithOpts(String topoName, String uploadedJarLocation,
topoConf.remove(Config.TOPOLOGY_CLASSPATH_BEGINNING);
}
+ // storm.messaging.netty.authentication is about inter-worker communication
Review comment:
I feel https://github.com/apache/storm/blob/master/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java#L1098 is a better place for this.
----------------------------------------------------------------
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:
users@infra.apache.org