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

    https://github.com/apache/spark/pull/22645#discussion_r223192937
  
    --- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/ui/AllExecutionsPage.scala
 ---
    @@ -121,65 +122,242 @@ private[ui] class AllExecutionsPage(parent: SQLTab) 
extends WebUIPage("") with L
               {
                 if (running.nonEmpty) {
                   <li>
    -                <a href="#running-execution-table"><strong>Running 
Queries:</strong></a>
    +                <a href="#running"><strong>Running Queries:</strong></a>
                     {running.size}
                   </li>
                 }
               }
               {
                 if (completed.nonEmpty) {
                   <li>
    -                <a href="#completed-execution-table"><strong>Completed 
Queries:</strong></a>
    +                <a href="#completed"><strong>Completed 
Queries:</strong></a>
                     {completed.size}
                   </li>
                 }
               }
               {
                 if (failed.nonEmpty) {
                   <li>
    -                <a href="#failed-execution-table"><strong>Failed 
Queries:</strong></a>
    +                <a href="#failed"><strong>Failed Queries:</strong></a>
                     {failed.size}
                   </li>
                 }
               }
             </ul>
           </div>
    +
         UIUtils.headerSparkPage(request, "SQL", summary ++ content, parent, 
Some(5000))
       }
    +
    +  private def executionsTable(
    +    request: HttpServletRequest,
    +    executionTag: String,
    +    executionData: Seq[SQLExecutionUIData],
    +    currentTime: Long,
    +    showRunningJobs: Boolean,
    +    showSucceededJobs: Boolean,
    +    showFailedJobs: Boolean): Seq[Node] = {
    +
    +    // stripXSS is called to remove suspicious characters used in XSS 
attacks
    +    val allParameters = request.getParameterMap.asScala.toMap.map { case 
(k, v) =>
    +      UIUtils.stripXSS(k) -> v.map(UIUtils.stripXSS).toSeq
    +    }
    +    val parameterOtherTable = 
allParameters.filterNot(_._1.startsWith(executionTag))
    +      .map(para => para._1 + "=" + para._2(0))
    +
    +    val parameterExecutionPage = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.page"))
    +    val parameterExecutionSortColumn = UIUtils.stripXSS(request.
    +      getParameter(s"$executionTag.sort"))
    +    val parameterExecutionSortDesc = 
UIUtils.stripXSS(request.getParameter(s"$executionTag.desc"))
    +    val parameterExecutionPageSize = UIUtils.stripXSS(request.
    +      getParameter(s"$executionTag.pageSize"))
    +    val parameterExecutionPrevPageSize = UIUtils.stripXSS(request.
    +      getParameter(s"$executionTag.prevPageSize"))
    +
    +    val executionPage = 
Option(parameterExecutionPage).map(_.toInt).getOrElse(1)
    +    val executionSortColumn = Option(parameterExecutionSortColumn).map { 
sortColumn =>
    +      UIUtils.decodeURLParameter(sortColumn)
    +    }.getOrElse("ID")
    +    val executionSortDesc = 
Option(parameterExecutionSortDesc).map(_.toBoolean).getOrElse(
    +      // New executions should be shown above old executions by default.
    +      executionSortColumn == "ID"
    +    )
    +    val executionPageSize = 
Option(parameterExecutionPageSize).map(_.toInt).getOrElse(100)
    +    val executionPrevPageSize = 
Option(parameterExecutionPrevPageSize).map(_.toInt).
    +      getOrElse(executionPageSize)
    +
    +    // If the user has changed to a larger page size, then go to page 1 in 
order to avoid
    +    // IndexOutOfBoundsException.
    +    val page: Int = if (executionPageSize <= executionPrevPageSize) {
    +      executionPage
    +    } else {
    +      1
    +    }
    +    val tableHeaderId = executionTag // "running", "completed" or "failed"
    +
    +    try {
    +      new ExecutionPagedTable(
    +        request,
    +        parent,
    +        executionData,
    +        tableHeaderId,
    +        executionTag,
    +        UIUtils.prependBaseUri(request, parent.basePath),
    +        "SQL", // subPath
    +        parameterOtherTable,
    +        currentTime,
    +        pageSize = executionPageSize,
    +        sortColumn = executionSortColumn,
    +        desc = executionSortDesc,
    +        showRunningJobs,
    +        showSucceededJobs,
    +        showFailedJobs).table(page)
    +    } catch {
    +      case e@(_: IllegalArgumentException | _: IndexOutOfBoundsException) 
=>
    +        <div class="alert alert-error">
    +          <p>Error while rendering execution table:</p>
    +          <pre>
    +            {Utils.exceptionString(e)}
    +          </pre>
    +        </div>
    +    }
    +  }
     }
     
    -private[ui] abstract class ExecutionTable(
    +
    +private[ui] class ExecutionPagedTable(
    +    request: HttpServletRequest,
         parent: SQLTab,
    -    tableId: String,
    +    data: Seq[SQLExecutionUIData],
    +    tableHeaderId: String,
    +    executionTag: String,
    +    basePath: String,
    +    subPath: String,
    +    parameterOtherTable: Iterable[String],
         currentTime: Long,
    -    executionUIDatas: Seq[SQLExecutionUIData],
    +    pageSize: Int,
    +    sortColumn: String,
    +    desc: Boolean,
         showRunningJobs: Boolean,
         showSucceededJobs: Boolean,
    -    showFailedJobs: Boolean) {
    +    showFailedJobs: Boolean) extends PagedTable[ExecutionTableRowData] {
    +
    +  override val dataSource = new ExecutionDataSource(
    +    request,
    +    parent,
    +    data,
    +    basePath,
    +    currentTime,
    +    pageSize,
    +    sortColumn,
    +    desc)
    +
    +  private val parameterPath = 
s"$basePath/$subPath/?${parameterOtherTable.mkString("&")}"
     
    -  protected def baseHeader: Seq[String] = Seq(
    -    "ID",
    -    "Description",
    -    "Submitted",
    -    "Duration")
    +  override def tableId: String = s"$executionTag-table"
     
    -  protected def header: Seq[String]
    +  override def tableCssClass: String =
    +    "table table-bordered table-condensed table-striped " +
    +      "table-head-clickable table-cell-width-limited"
     
    -  protected def row(
    -      request: HttpServletRequest,
    -      currentTime: Long,
    -      executionUIData: SQLExecutionUIData): Seq[Node] = {
    +  override def prevPageSizeFormField: String = 
s"$executionTag.prevPageSize"
    +
    +  override def pageLink(page: Int): String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    parameterPath +
    +      s"&$pageNumberFormField=$page" +
    +      s"&$executionTag.sort=$encodedSortColumn" +
    +      s"&$executionTag.desc=$desc" +
    +      s"&$pageSizeFormField=$pageSize" +
    +      s"#$tableHeaderId"
    +  }
    +
    +  override def pageSizeFormField: String = s"$executionTag.pageSize"
    +
    +  override def pageNumberFormField: String = s"$executionTag.page"
    +
    +  override def goButtonFormPath: String = {
    +    val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
    +    
s"$parameterPath&$executionTag.sort=$encodedSortColumn&$executionTag.desc=$desc#$tableHeaderId"
    +  }
    +
    +  override def headers: Seq[Node] = {
    +    // Information for each header: title, cssClass, and sortable
    +    val executionHeadersAndCssClasses: Seq[(String, String, Boolean)] =
    +      Seq(("ID", "", true), ("Description", "", true), ("Submitted", "", 
true),
    --- End diff --
    
    This might be a good place to wrap these to line up the elements in the 
Seq, one one each line (?) to make it easier to read its contents. They all 
seem to have an empty string as the second element? can it be removed or did I 
overlook something? Also you don't need the type here.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

Reply via email to