Github user pwendell commented on a diff in the pull request:

    https://github.com/apache/spark/pull/33#discussion_r10151554
  
    --- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
    @@ -41,56 +46,103 @@ private[spark] object JettyUtils extends Logging {
       type Responder[T] = HttpServletRequest => T
     
       // Conversions from various types of Responder's to jetty Handlers
    -  implicit def jsonResponderToHandler(responder: Responder[JValue]): 
Handler =
    +  implicit def jsonResponderToHandler(responder: Responder[JValue]): 
HttpServlet =
         createHandler(responder, "text/json", (in: JValue) => 
pretty(render(in)))
     
    -  implicit def htmlResponderToHandler(responder: Responder[Seq[Node]]): 
Handler =
    +  implicit def htmlResponderToHandler(responder: Responder[Seq[Node]]): 
HttpServlet =
         createHandler(responder, "text/html", (in: Seq[Node]) => "<!DOCTYPE 
html>" + in.toString)
     
    -  implicit def textResponderToHandler(responder: Responder[String]): 
Handler =
    +  implicit def textResponderToHandler(responder: Responder[String]): 
HttpServlet =
         createHandler(responder, "text/plain")
     
    -  def createHandler[T <% AnyRef](responder: Responder[T], contentType: 
String,
    -                                 extractFn: T => String = (in: Any) => 
in.toString): Handler = {
    -    new AbstractHandler {
    -      def handle(target: String,
    -                 baseRequest: Request,
    -                 request: HttpServletRequest,
    +  def createHandler[T <% AnyRef](responder: Responder[T], contentType: 
String, 
    +                                 extractFn: T => String = (in: Any) => 
in.toString): HttpServlet = {
    +    new HttpServlet {
    +      override def doGet(request: HttpServletRequest,
                      response: HttpServletResponse) {
    -        response.setContentType("%s;charset=utf-8".format(contentType))
    -        response.setStatus(HttpServletResponse.SC_OK)
    -        baseRequest.setHandled(true)
    -        val result = responder(request)
    -        response.setHeader("Cache-Control", "no-cache, no-store, 
must-revalidate")
    -        response.getWriter().println(extractFn(result))
    +        // First try to get the security Manager from the SparkEnv. If 
that doesn't exist, create
    +        // a new one and rely on the configs being set
    +        val sparkEnv = SparkEnv.get
    +        val securityMgr = if (sparkEnv != null) sparkEnv.securityManager 
else new SecurityManager()
    --- End diff --
    
    Ideally the way this works is that we would just pass a security manager to 
this function rather than initializing some configuration inside of a static 
utility like this.
    
    The model going forward with SparkConf is that all components should 
configure themselves via a SparkConf and we shouldn't directly use system 
properties or thread locals to do various configurations.
    
    So the ideal way this would work is that we create a single SecurityManager 
inside the Master and Worker classes (based on the SparkConf they each create) 
and then that gets passed into all calls of these static utilities. For the 
UI's inside of an active SparkContext we'd just pass `sc.env.securityManager`. 
    
    The downside is it will make using these functions a little clunkier 
because really you need a reference to the security manager for every handler 
you create. That's a bit clunkier but ultimately better (I think). Doing this 
initialization inside of each servlet isn't so good in the SparkConf world.


---
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.
---

Reply via email to