You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by catap <gi...@git.apache.org> on 2015/01/28 08:34:45 UTC
[GitHub] spark pull request: Don't return `ERROR 500` when have missing arg...
GitHub user catap opened a pull request:
https://github.com/apache/spark/pull/4239
Don't return `ERROR 500` when have missing args
Spark web UI return `HTTP ERROR 500` when GET arguments is missing.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/catap/spark ui_500
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/4239.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 #4239
----
commit 4faba92526e93f44c11962724180e8e201015e7a
Author: Kirill A. Korinskiy <ca...@catap.ru>
Date: 2015-01-28T07:26:55Z
Don't return `ERROR 500` when have missing args
Spark web UI return `HTTP ERROR 500` when GET arguments is missing.
----
---
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: Don't return `ERROR 500` when have missing arg...
Posted by catap <gi...@git.apache.org>.
Github user catap commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r24290424
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala ---
@@ -32,7 +32,12 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
def render(request: HttpServletRequest): Seq[Node] = {
listener.synchronized {
- val jobId = request.getParameter("id").toInt
+ val parameterId = request.getParameter("id")
--- End diff --
Yes, you're right, but `parameterId.nonEmpty` is more clear, isn't it?
---
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: Don't return `ERROR 500` when have missing arg...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-71913853
ok to test
---
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: Don't return `ERROR 500` when have missing arg...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-72292000
[Test build #26436 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26436/consoleFull) for PR 4239 at commit [`b6066e3`](https://github.com/apache/spark/commit/b6066e3d72e95b1c0e53ddc84ad8772a4d5d8a09).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r23679494
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala ---
@@ -32,7 +32,12 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
def render(request: HttpServletRequest): Seq[Node] = {
listener.synchronized {
- val jobId = request.getParameter("id").toInt
+ val paramaterId = request.getParameter("id")
--- End diff --
Nit: paramater -> parameter
---
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: Don't return `ERROR 500` when have missing arg...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r23680215
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala ---
@@ -32,7 +32,12 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
def render(request: HttpServletRequest): Seq[Node] = {
listener.synchronized {
- val jobId = request.getParameter("id").toInt
+ val paramaterId = request.getParameter("id")
+ if (paramaterId.equals("")) {
--- End diff --
@catap Hm, no it looks like it is in fact `null`: http://docs.oracle.com/javaee/6/api/javax/servlet/ServletRequest.html#getParameter(java.lang.String)
---
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: Don't return `ERROR 500` when have missing arg...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-72608720
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26623/
Test PASSed.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r23700874
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala ---
@@ -32,7 +32,12 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
def render(request: HttpServletRequest): Seq[Node] = {
listener.synchronized {
- val jobId = request.getParameter("id").toInt
+ val paramaterId = request.getParameter("id")
+ if (paramaterId.equals("")) {
--- End diff --
Yes, but don't you want the code to handle both cases? and test both cases?
On Wed, Jan 28, 2015 at 4:44 PM, Kirill A. Korinskiy <
notifications@github.com> wrote:
> In core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala
> <https://github.com/apache/spark/pull/4239#discussion_r23700631>:
>
> > @@ -32,7 +32,12 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
> >
> > def render(request: HttpServletRequest): Seq[Node] = {
> > listener.synchronized {
> > - val jobId = request.getParameter("id").toInt
> > + val paramaterId = request.getParameter("id")
> > + if (paramaterId.equals("")) {
>
> @srowen <https://github.com/srowen>, yes, you are right.
>
> But for case &id= it's return "". And I used this case for testing.
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/apache/spark/pull/4239/files#r23700631>.
>
---
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: Don't return `ERROR 500` when have missing arg...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r24290178
--- Diff: core/src/main/scala/org/apache/spark/ui/exec/ExecutorThreadDumpPage.scala ---
@@ -20,11 +20,10 @@ package org.apache.spark.ui.exec
import java.net.URLDecoder
import javax.servlet.http.HttpServletRequest
-import scala.util.Try
-import scala.xml.{Text, Node}
-
import org.apache.spark.ui.{UIUtils, WebUIPage}
+import scala.xml.{Node, Text}
--- End diff --
These imports were correct where they were I believe.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-72602839
[Test build #26623 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26623/consoleFull) for PR 4239 at commit [`ed4154d`](https://github.com/apache/spark/commit/ed4154d1df85bc5a790e1c078d0891f6dd5619a3).
* This patch merges cleanly.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by catap <gi...@git.apache.org>.
Github user catap commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r24290358
--- Diff: core/src/main/scala/org/apache/spark/ui/exec/ExecutorThreadDumpPage.scala ---
@@ -20,11 +20,10 @@ package org.apache.spark.ui.exec
import java.net.URLDecoder
import javax.servlet.http.HttpServletRequest
-import scala.util.Try
-import scala.xml.{Text, Node}
-
import org.apache.spark.ui.{UIUtils, WebUIPage}
+import scala.xml.{Node, Text}
--- End diff --
I use Intellij IDEA and this was changed 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: Don't return `ERROR 500` when have missing arg...
Posted by catap <gi...@git.apache.org>.
Github user catap commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r23701196
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala ---
@@ -32,7 +32,12 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
def render(request: HttpServletRequest): Seq[Node] = {
listener.synchronized {
- val jobId = request.getParameter("id").toInt
+ val paramaterId = request.getParameter("id")
+ if (paramaterId.equals("")) {
--- End diff --
Yes, I updated the code to handle both cases.
BTW I test it only by hand because I not found any testcase for HTTP UI.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-72292005
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26436/
Test FAILed.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-73383481
It's looking good to me but can you make a JIRA (https://issues.apache.org/jira/browse/SPARK/) and put it into the title like `SPARK-xxxx [WEB UI] ...` ?
---
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: Don't return `ERROR 500` when have missing arg...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-73374959
[Test build #27004 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27004/consoleFull) for PR 4239 at commit [`3ba74ba`](https://github.com/apache/spark/commit/3ba74bab99617e0f9c55d0f6e104113e0b1314a3).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* ` case class MasterStateResponse(`
* `class LocalSparkCluster(`
* ` * (4) the main class for the child`
* ` case class BoundPortsResponse(actorPort: Int, webUIPort: Int, restPort: Option[Int])`
* ` throw new SubmitRestMissingFieldException("Main class is missing.")`
---
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: Don't return `ERROR 500` when have missing arg...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-73371613
[Test build #27004 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27004/consoleFull) for PR 4239 at commit [`3ba74ba`](https://github.com/apache/spark/commit/3ba74bab99617e0f9c55d0f6e104113e0b1314a3).
* This patch merges cleanly.
---
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-5672][Web UI] Don't return `ERROR 500` ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-73392947
[Test build #27020 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27020/consoleFull) for PR 4239 at commit [`520e180`](https://github.com/apache/spark/commit/520e180580325652e4e9b966326d778dc379b881).
* This patch merges cleanly.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-72608713
[Test build #26623 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26623/consoleFull) for PR 4239 at commit [`ed4154d`](https://github.com/apache/spark/commit/ed4154d1df85bc5a790e1c078d0891f6dd5619a3).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by catap <gi...@git.apache.org>.
Github user catap commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r23700631
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala ---
@@ -32,7 +32,12 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
def render(request: HttpServletRequest): Seq[Node] = {
listener.synchronized {
- val jobId = request.getParameter("id").toInt
+ val paramaterId = request.getParameter("id")
+ if (paramaterId.equals("")) {
--- End diff --
@srowen, yes, you are right.
But for case `&id=` it's return "". And I used this case for testing.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-72286653
[Test build #26436 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/26436/consoleFull) for PR 4239 at commit [`b6066e3`](https://github.com/apache/spark/commit/b6066e3d72e95b1c0e53ddc84ad8772a4d5d8a09).
* This patch merges cleanly.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-72286357
retest this please
---
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-5672][Web UI] Don't return `ERROR 500` ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-73394850
[Test build #27020 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27020/consoleFull) for PR 4239 at commit [`520e180`](https://github.com/apache/spark/commit/520e180580325652e4e9b966326d778dc379b881).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r24290438
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala ---
@@ -32,7 +32,12 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
def render(request: HttpServletRequest): Seq[Node] = {
listener.synchronized {
- val jobId = request.getParameter("id").toInt
+ val parameterId = request.getParameter("id")
--- End diff --
Ah right, you can use that in Scala. Yes.
---
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-5672][Web UI] Don't return `ERROR 500` ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/4239
---
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: Don't return `ERROR 500` when have missing arg...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r23679510
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala ---
@@ -32,7 +32,12 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
def render(request: HttpServletRequest): Seq[Node] = {
listener.synchronized {
- val jobId = request.getParameter("id").toInt
+ val paramaterId = request.getParameter("id")
+ if (paramaterId.equals("")) {
--- End diff --
It is `null` if not specified, right?
---
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: Don't return `ERROR 500` when have missing arg...
Posted by catap <gi...@git.apache.org>.
Github user catap commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-71815969
Ok, the patch was updated.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r24290191
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala ---
@@ -32,7 +32,12 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
def render(request: HttpServletRequest): Seq[Node] = {
listener.synchronized {
- val jobId = request.getParameter("id").toInt
+ val parameterId = request.getParameter("id")
--- End diff --
Elsewhere the code uses `require(condition, message)` to generate an `IllegalArgumentException`. I think that could be tidier here. You can also use `isEmpty`:
`require(parameterId != null && !parameterId.isEmpty, "Missing id parameter")`
---
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: Don't return `ERROR 500` when have missing arg...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-71916931
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/26247/
Test FAILed.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-71792811
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: Don't return `ERROR 500` when have missing arg...
Posted by catap <gi...@git.apache.org>.
Github user catap commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r23679953
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/JobPage.scala ---
@@ -32,7 +32,12 @@ private[ui] class JobPage(parent: JobsTab) extends WebUIPage("job") {
def render(request: HttpServletRequest): Seq[Node] = {
listener.synchronized {
- val jobId = request.getParameter("id").toInt
+ val paramaterId = request.getParameter("id")
+ if (paramaterId.equals("")) {
--- End diff --
Nope, is return "" when not specified.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-73374961
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27004/
Test PASSed.
---
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-5672][Web UI] Don't return `ERROR 500` ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-73394852
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27020/
Test PASSed.
---
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: Don't return `ERROR 500` when have missing arg...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4239#discussion_r24290182
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/PoolPage.scala ---
@@ -19,10 +19,10 @@ package org.apache.spark.ui.jobs
import javax.servlet.http.HttpServletRequest
-import scala.xml.Node
+import org.apache.spark.scheduler.StageInfo
+import org.apache.spark.ui.{UIUtils, WebUIPage}
-import org.apache.spark.scheduler.{Schedulable, StageInfo}
-import org.apache.spark.ui.{WebUIPage, UIUtils}
+import scala.xml.Node
--- End diff --
Ditto, I think we put scala after java imports and before other stuff
---
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: Don't return `ERROR 500` when have missing arg...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/4239#issuecomment-71801048
I agree that 5xx means server error, and is not an appropriate response. But assuming a value of -1 doesn't seem like a great solution -- how about returning `400 Bad Request`?
---
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