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 [email protected] or file a JIRA ticket
with INFRA.
---
