Github user pwendell commented on the pull request: https://github.com/apache/spark/pull/33#issuecomment-36315155 Hey Tom, This patch is looking really great. I took a pretty thorough look through this and there were really only two high level things: 1. Configuration. The model we've move towards is that a SparkContext is configured by a SparkConf and that Conf is used to configure all components. For the master/worker they each instantiate a conf as well. It would be good if instead of system properties and environment variables, a security manager takes a SparkConf on initialization. And other parts of the code use SparkConf as well. If it seems like this isn't possible anywhere, let me know, that might represent a shortfall in the current design of SparkConf. 2. Default behavior and logging. Thinking a bit about environments less homogeneous than Yahoo, we should try to minimize the potential for confusion and mistakes when configuring security. So I was thinking maybe if users, e.g., don't set spark secret we should throw an exception, and suggested logging more detail about the security set-up at info level. Some other smaller comments as well.
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---