Fixed PR comments.

apache · Jan 14, 2016 · 384b990 · 384b990
1 parent 3d508c6
commit 384b990
Show file tree

Hide file tree

Showing 4 changed files with 16 additions and 10 deletions.
diff --git a/core/src/main/scala/org/apache/spark/ui/exec/ExecutorThreadDumpPage.scala b/core/src/main/scala/org/apache/spark/ui/exec/ExecutorThreadDumpPage.scala
@@ -29,7 +29,9 @@ private[ui] class ExecutorThreadDumpPage(parent: ExecutorsTab) extends WebUIPage
   private val sc = parent.sc
 
   def render(request: HttpServletRequest): Seq[Node] = {
-    val executorId = Option(request.getParameter("executorId")).getOrElse {
+    val executorId = Option(request.getParameter("executorId")).map { executorId =>
+      UIUtils.decodeURLParameter(executorId)
+    }.getOrElse {
       throw new IllegalArgumentException(s"Missing executorId parameter")
     }
     val time = System.currentTimeMillis()

diff --git a/core/src/main/scala/org/apache/spark/ui/jobs/PoolPage.scala b/core/src/main/scala/org/apache/spark/ui/jobs/PoolPage.scala
@@ -31,8 +31,11 @@ private[ui] class PoolPage(parent: StagesTab) extends WebUIPage("pool") {
 
   def render(request: HttpServletRequest): Seq[Node] = {
     listener.synchronized {
-      val poolName = UIUtils.decodeURLParameter(request.getParameter("poolname"))
-      require(poolName != null && poolName.nonEmpty, "Missing poolname parameter")
+      val poolName = Option(request.getParameter("poolname")).map { poolname =>
+        UIUtils.decodeURLParameter(poolname)
+      }.getOrElse {
+        throw new IllegalArgumentException(s"Missing poolname parameter")
+      }
 
       val poolToActiveStages = listener.poolToActiveStages
       val activeStages = poolToActiveStages.get(poolName) match {
@@ -44,7 +47,9 @@ private[ui] class PoolPage(parent: StagesTab) extends WebUIPage("pool") {
         killEnabled = parent.killEnabled)
 
       // For now, pool information is only accessible in live UIs
-      val pools = sc.map(_.getPoolForName(poolName).get).toSeq
+      val pools = sc.map(_.getPoolForName(poolName).getOrElse {
+        throw new IllegalArgumentException(s"Unknown poolname: $poolName")
+      }).toSeq
       val poolTable = new PoolTable(pools, parent)
 
       val content =

diff --git a/core/src/main/scala/org/apache/spark/ui/jobs/PoolTable.scala b/core/src/main/scala/org/apache/spark/ui/jobs/PoolTable.scala
@@ -61,8 +61,7 @@ private[ui] class PoolTable(pools: Seq[Schedulable], parent: StagesTab) {
       case None => 0
     }
     val href = "%s/stages/pool?poolname=%s"
-      .format(UIUtils.prependBaseUri(parent.basePath),
-        URLEncoder.encode(p.name, "UTF-8"))
+      .format(UIUtils.prependBaseUri(parent.basePath), URLEncoder.encode(p.name, "UTF-8"))
     <tr>
       <td>
         <a href={href}>{p.name}</a>

diff --git a/core/src/test/scala/org/apache/spark/ui/UIUtilsSuite.scala b/core/src/test/scala/org/apache/spark/ui/UIUtilsSuite.scala
@@ -73,12 +73,12 @@ class UIUtilsSuite extends SparkFunSuite {
     val encoded2 = "%253Cdriver%253E"
     val decoded2 = "<driver>"
 
-    assert(decoded1.sameElements(decodeURLParameter(encoded1)))
-    assert(decoded2.sameElements(decodeURLParameter(encoded2)))
+    assert(decoded1 === decodeURLParameter(encoded1))
+    assert(decoded2 === decodeURLParameter(encoded2))
 
     // verify that no affect to decoded URL.
-    assert(decoded1.sameElements(decodeURLParameter(decoded1)))
-    assert(decoded2.sameElements(decodeURLParameter(decoded2)))
+    assert(decoded1 === decodeURLParameter(decoded1))
+    assert(decoded2 === decodeURLParameter(decoded2))
   }
 
   private def verify(