You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by XuTingjun <gi...@git.apache.org> on 2014/08/21 03:58:34 UTC
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
GitHub user XuTingjun opened a pull request:
https://github.com/apache/spark/pull/2073
[SPARK-3168]The ServletContextHandler of webui lacks a SessionManager
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/XuTingjun/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/2073.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2073
----
commit f78ce16c292a45a485b42c932e0d7bdc1007cc91
Author: meiyoula <10...@qq.com>
Date: 2014-08-21T01:55:32Z
Update HistoryServer.scala
commit 1e0ec23099d831aab9a12a5ad2fa1fc9b9adcb7e
Author: meiyoula <10...@qq.com>
Date: 2014-08-21T01:57:27Z
Update JettyUtils.scala
----
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by XuTingjun <gi...@git.apache.org>.
Github user XuTingjun closed the pull request at:
https://github.com/apache/spark/pull/2073
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by XuTingjun <gi...@git.apache.org>.
Github user XuTingjun commented on the pull request:
https://github.com/apache/spark/pull/2073#issuecomment-55088380
Now Spark doesn't support SSL
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/2073#issuecomment-54399248
I'm not an expert in Sessions. My main concern would be that the config are setup properly and to be secure. For instance with the HashSessionManager does it default to the httponly, is it using secure cookies, etc. Do you know what the default settings are for some of these?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2073#issuecomment-54694431
Can one of the admins verify this patch?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/2073#discussion_r16628942
--- Diff: core/src/main/scala/org/apache/spark/ui/JettyUtils.scala ---
@@ -35,6 +35,7 @@ import org.json4s.jackson.JsonMethods.{pretty, render}
import org.apache.spark.{Logging, SecurityManager, SparkConf}
import org.apache.spark.util.Utils
+import org.eclipse.jetty.server.session.{HashSessionManager, SessionHandler}
--- End diff --
Organize this with the other jetty imports
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/2073#discussion_r16628944
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -28,6 +28,7 @@ import org.apache.spark.deploy.SparkHadoopUtil
import org.apache.spark.ui.{SparkUI, UIUtils, WebUI}
import org.apache.spark.ui.JettyUtils._
import org.apache.spark.util.SignalLogger
+import org.eclipse.jetty.server.session.{HashSessionManager, SessionHandler}
--- End diff --
organize this with the other jetty imports.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by vanzin <gi...@git.apache.org>.
Github user vanzin commented on the pull request:
https://github.com/apache/spark/pull/2073#issuecomment-55035316
I'd be more comfortable with enabling sessions based on a config option; sessions are not needed for the built-in functionality, and they do incur overhead (aside from all the questions @tgravescs asked above).
One thing I'd do, not that I think it matters much here, is to restrict the session cookies to the protocol used:
if (sslEnabled) {
sessionManager.getSessionCookieConfig().setSecure(true);
} else {
sessionManager.getSessionCookieConfig().setHttpOnly(true);
}
(Does Spark even support SSL?)
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by tgravescs <gi...@git.apache.org>.
Github user tgravescs commented on the pull request:
https://github.com/apache/spark/pull/2073#issuecomment-54486981
Note that I'm assuming some of these don't necessarily apply. Like I think secure cookies can only be used over https. Looking a bit more at the api I think I am interested in.
- make sure we don't save it to disk, unless we are sure its going to a correct and secure location
- what is the invalidate timeout? I assume CAS will handle relogin when session expires?
- what is the max number of sessions?
- what is the httponly setting - I think we should set it unless it causes other issues for CAS.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/2073#issuecomment-52869862
Can one of the admins verify this patch?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/2073#issuecomment-75157134
@XuTingjun can you elaborate on the requirements of CAS? Are there performance costs to using a HashSessionHandler here, since it saves the session to disk periodically (IIUC)? There also seems to be a few questions that are raised by @tgravescs but are not addressed.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/2073#discussion_r16628946
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -116,6 +117,8 @@ class HistoryServer(
val contextHandler = new ServletContextHandler
contextHandler.setContextPath(HistoryServer.UI_PATH_PREFIX)
contextHandler.addServlet(new ServletHolder(loaderServlet), "/*")
+ val sessionHandler = new SessionHandler(new HashSessionManager())
--- End diff --
@rxin does this look okay to you? I'm no jetty expert, but it seems like a reasonable change.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3168]The ServletContextHandler of webui...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/2073#discussion_r16811373
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -116,6 +117,8 @@ class HistoryServer(
val contextHandler = new ServletContextHandler
contextHandler.setContextPath(HistoryServer.UI_PATH_PREFIX)
contextHandler.addServlet(new ServletHolder(loaderServlet), "/*")
+ val sessionHandler = new SessionHandler(new HashSessionManager())
--- End diff --
I asked a question on JIRA ... https://issues.apache.org/jira/browse/SPARK-3168
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org