You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by JoshRosen <gi...@git.apache.org> on 2015/12/22 23:00:40 UTC
[GitHub] spark pull request: [SPARK-12490] Don't use Javascript for web UI'...
GitHub user JoshRosen opened a pull request:
https://github.com/apache/spark/pull/10441
[SPARK-12490] Don't use Javascript for web UI's paginated table controls
The web UI's paginated table uses Javascript to implement certain navigation controls, such as table sorting and the "go to page" form. This is unnecessary and should be simplified to use plain HTML form controls and links.
/cc @zsxwing, who wrote this original code, and @yhuai.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/JoshRosen/spark simplify-paginated-table-sorting
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/10441.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 #10441
----
commit 00292fa60722dfc340843bac754e2875b0b9fa0c
Author: Josh Rosen <jo...@databricks.com>
Date: 2015-12-22T20:41:40Z
Remove use of JS for table header sorting.
commit 61efa4fdd2d4535f3f2f4c5172f8af1628dfafbb
Author: Josh Rosen <jo...@databricks.com>
Date: 2015-12-22T20:44:38Z
Apply the same fix to RDDPage.
commit 970be95e20789f24f30f4e378f8ff90865c4a2e2
Author: Josh Rosen <jo...@databricks.com>
Date: 2015-12-22T21:56:56Z
Remove use of Javascript for form pagination form controls.
----
---
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-12490] Don't use Javascript for web UI'...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166756541
Note that my exception changes were actually motivated by a case where this error-handling logic swallowed other internal exceptions and only printed "1" as output. The problem is that the `try-catch` blocks here are capable of intercepting exceptions which occur as a result of internal programming errors / violated assumptions, not just bad user input.
---
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-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167675687
Merged build finished. 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: [SPARK-12490] Don't use Javascript for web UI'...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10441#discussion_r48303869
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -1232,24 +1243,16 @@ private[ui] class TaskPagedTable(
override def pageLink(page: Int): String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- s"${basePath}&task.page=$page&task.sort=${encodedSortColumn}&task.desc=${desc}" +
- s"&task.pageSize=${pageSize}"
+ basePath +
+ s"&$pageNumberFormField=$page" +
+ s"&task.sort=$encodedSortColumn" +
+ s"&task.desc=$desc" +
+ s"&$pageSizeFormField=$pageSize"
}
- override def goButtonJavascriptFunction: (String, String) = {
- val jsFuncName = "goToTaskPage"
+ override def goButtonFormPath: String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- val jsFunc = s"""
- |currentTaskPageSize = ${pageSize}
- |function goToTaskPage(page, pageSize) {
- | // Set page to 1 if the page size changes
--- End diff --
TODO: I'm not sure if I properly preserved this behavior. I should write a Selenium test for this.
---
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-12490] Don't use Javascript for web UI'...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166795272
**[Test build #48227 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48227/consoleFull)** for PR 10441 at commit [`89827a5`](https://github.com/apache/spark/commit/89827a505f51ffce64d03f6115e7d9d801ca7ead).
---
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-12490] Don't use Javascript for web UI'...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166764008
**[Test build #48216 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48216/consoleFull)** for PR 10441 at commit [`970be95`](https://github.com/apache/spark/commit/970be95e20789f24f30f4e378f8ff90865c4a2e2).
* This patch **fails PySpark 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: [SPARK-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167675690
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48377/
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: [SPARK-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166777564
Merged build finished. 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-12490] Don't use Javascript for web UI'...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166746163
**[Test build #48216 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48216/consoleFull)** for PR 10441 at commit [`970be95`](https://github.com/apache/spark/commit/970be95e20789f24f30f4e378f8ff90865c4a2e2).
---
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-12490] Don't use Javascript for web UI'...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10441#discussion_r48314324
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -1232,24 +1243,16 @@ private[ui] class TaskPagedTable(
override def pageLink(page: Int): String = {
--- End diff --
Yeah, it technically works because browsers are pretty resilient but it's a little ugly when reading the generated HTML.
---
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-12490] Don't use Javascript for web UI'...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166777114
**[Test build #48221 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48221/consoleFull)** for PR 10441 at commit [`970be95`](https://github.com/apache/spark/commit/970be95e20789f24f30f4e378f8ff90865c4a2e2).
* 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: [SPARK-12490] Don't use Javascript for web UI'...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10441#discussion_r48323623
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -1232,24 +1243,16 @@ private[ui] class TaskPagedTable(
override def pageLink(page: Int): String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- s"${basePath}&task.page=$page&task.sort=${encodedSortColumn}&task.desc=${desc}" +
- s"&task.pageSize=${pageSize}"
+ basePath +
+ s"&$pageNumberFormField=$page" +
+ s"&task.sort=$encodedSortColumn" +
+ s"&task.desc=$desc" +
+ s"&$pageSizeFormField=$pageSize"
}
- override def goButtonJavascriptFunction: (String, String) = {
- val jsFuncName = "goToTaskPage"
+ override def goButtonFormPath: String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- val jsFunc = s"""
- |currentTaskPageSize = ${pageSize}
- |function goToTaskPage(page, pageSize) {
- | // Set page to 1 if the page size changes
--- End diff --
Could we handle this on the server-side by detecting this case, automatically setting the effective page number back to 1, and displaying a warning to the user?
---
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-12490] Don't use Javascript for web UI'...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167684883
**[Test build #48378 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48378/consoleFull)** for PR 10441 at commit [`fd2d1f2`](https://github.com/apache/spark/commit/fd2d1f2a49ad4f6bc2b5ed8bf3aecd65093abc65).
* 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: [SPARK-12490] Don't use Javascript for web UI'...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10441#discussion_r48314294
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -1232,24 +1243,16 @@ private[ui] class TaskPagedTable(
override def pageLink(page: Int): String = {
--- End diff --
Looks the browser somehow handles the re-encoded urls correctly.
---
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-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166764047
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48216/
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: [SPARK-12490] Don't use Javascript for web UI'...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167685127
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-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167688408
Merged build finished. 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: [SPARK-12490] Don't use Javascript for web UI'...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166765190
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-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166764045
Merged build finished. 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: [SPARK-12490] Don't use Javascript for web UI'...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10441#discussion_r48305560
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -1232,24 +1243,16 @@ private[ui] class TaskPagedTable(
override def pageLink(page: Int): String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- s"${basePath}&task.page=$page&task.sort=${encodedSortColumn}&task.desc=${desc}" +
- s"&task.pageSize=${pageSize}"
+ basePath +
+ s"&$pageNumberFormField=$page" +
+ s"&task.sort=$encodedSortColumn" +
+ s"&task.desc=$desc" +
+ s"&$pageSizeFormField=$pageSize"
}
- override def goButtonJavascriptFunction: (String, String) = {
- val jsFuncName = "goToTaskPage"
+ override def goButtonFormPath: String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- val jsFunc = s"""
- |currentTaskPageSize = ${pageSize}
- |function goToTaskPage(page, pageSize) {
- | // Set page to 1 if the page size changes
--- End diff --
This is not preserved. We need to add a new parameter in the url to support it.However, it's only a tiny issue. Maybe not worth to fix it. The worst case is the user changes to a bigger page size and encounters `IndexOutOfBoundsException`.
BTW, I used JS previously was because of this issue.
---
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-12490] Don't use Javascript for web UI'...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10441#discussion_r48305122
--- Diff: core/src/main/scala/org/apache/spark/ui/PagedTable.scala ---
@@ -95,7 +106,12 @@ private[ui] trait PagedTable[T] {
val PageData(totalPages, _) = _dataSource.pageData(1)
<div>
{pageNavigation(1, _dataSource.pageSize, totalPages)}
- <div class="alert alert-error">{e.getMessage}</div>
+ <div class="alert alert-error">
+ <p>Error while rendering table:</p>
+ <pre>
+ {Utils.exceptionString(e)}
--- End diff --
This is just for `IndexOutOfBoundsException`. Using `e.getMessage` would be enough. `Utils.exceptionString` is really not friendly, e.g.,
<img width="1347" alt="screen shot 2015-12-22 at 2 21 14 pm" src="https://cloud.githubusercontent.com/assets/1000778/11967403/615c6b9e-a8b7-11e5-800c-1130bb7ec7c5.png">
---
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-12490] Don't use Javascript for web UI'...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166765248
> Note that my exception changes were actually motivated by a case where this error-handling logic swallowed other internal exceptions and only printed "1" as output. The problem is that the try-catch blocks here are capable of intercepting exceptions which occur as a result of internal programming errors / violated assumptions, not just bad user input.
Okey. Let's print the whole stack trace. LGTM
---
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-12490] Don't use Javascript for web UI'...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10441#discussion_r48324440
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -1232,24 +1243,16 @@ private[ui] class TaskPagedTable(
override def pageLink(page: Int): String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- s"${basePath}&task.page=$page&task.sort=${encodedSortColumn}&task.desc=${desc}" +
- s"&task.pageSize=${pageSize}"
+ basePath +
+ s"&$pageNumberFormField=$page" +
+ s"&task.sort=$encodedSortColumn" +
+ s"&task.desc=$desc" +
+ s"&$pageSizeFormField=$pageSize"
}
- override def goButtonJavascriptFunction: (String, String) = {
- val jsFuncName = "goToTaskPage"
+ override def goButtonFormPath: String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- val jsFunc = s"""
- |currentTaskPageSize = ${pageSize}
- |function goToTaskPage(page, pageSize) {
- | // Set page to 1 if the page size changes
--- End diff --
We need one more parameter to detect the change of `pageSize`.
---
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-12490] Don't use Javascript for web UI'...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/10441
---
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-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167684909
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48378/
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: [SPARK-12490] Don't use Javascript for web UI'...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167675703
Jenkins, 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-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167684908
Merged build finished. 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: [SPARK-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167688409
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48383/
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: [SPARK-12490] Don't use Javascript for web UI'...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167685120
The relevant UI tests all passed and the only failures were due to a known flaky test / build executor, so I'm going to merge this into master. Thanks!
---
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-12490] Don't use Javascript for web UI'...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10441#discussion_r48313993
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -1232,24 +1243,16 @@ private[ui] class TaskPagedTable(
override def pageLink(page: Int): String = {
--- End diff --
One problem: the URL that this returns is already encoded, but it ends up being re-encoded when Scala XML substitutes it, so the ampersands wind up as `&` in the generated HTML. I'll fix this.
---
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-12490] Don't use Javascript for web UI'...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:
https://github.com/apache/spark/pull/10441#discussion_r48305789
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -295,7 +295,14 @@ private[ui] class StagePage(parent: StagesTab) extends WebUIPage("stage") {
(_taskTable, _taskTable.table(taskPage))
} catch {
case e @ (_ : IllegalArgumentException | _ : IndexOutOfBoundsException) =>
- (null, <div class="alert alert-error">{e.getMessage}</div>)
+ val errorMessage =
+ <div class="alert alert-error">
+ <p>Error while rendering stage table:</p>
+ <pre>
+ {Utils.exceptionString(e)}
--- End diff --
Same as my previous comment about `Utils.exceptionString`
---
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-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166806883
Merged build finished. 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-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166777567
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48221/
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-12490] Don't use Javascript for web UI'...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167677083
**[Test build #48378 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48378/consoleFull)** for PR 10441 at commit [`fd2d1f2`](https://github.com/apache/spark/commit/fd2d1f2a49ad4f6bc2b5ed8bf3aecd65093abc65).
---
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-12490] Don't use Javascript for web UI'...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166806884
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48227/
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-12490] Don't use Javascript for web UI'...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10441#discussion_r48511839
--- Diff: core/src/main/scala/org/apache/spark/ui/jobs/StagePage.scala ---
@@ -1232,24 +1243,16 @@ private[ui] class TaskPagedTable(
override def pageLink(page: Int): String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- s"${basePath}&task.page=$page&task.sort=${encodedSortColumn}&task.desc=${desc}" +
- s"&task.pageSize=${pageSize}"
+ basePath +
+ s"&$pageNumberFormField=$page" +
+ s"&task.sort=$encodedSortColumn" +
+ s"&task.desc=$desc" +
+ s"&$pageSizeFormField=$pageSize"
}
- override def goButtonJavascriptFunction: (String, String) = {
- val jsFuncName = "goToTaskPage"
+ override def goButtonFormPath: String = {
val encodedSortColumn = URLEncoder.encode(sortColumn, "UTF-8")
- val jsFunc = s"""
- |currentTaskPageSize = ${pageSize}
- |function goToTaskPage(page, pageSize) {
- | // Set page to 1 if the page size changes
--- End diff --
Done, see latest commit.
---
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-12490] Don't use Javascript for web UI'...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166766454
**[Test build #48221 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48221/consoleFull)** for PR 10441 at commit [`970be95`](https://github.com/apache/spark/commit/970be95e20789f24f30f4e378f8ff90865c4a2e2).
---
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-12490] Don't use Javascript for web UI'...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167672868
@zsxwing, I've pushed a new commit which aims to preserve the old behavior when increasing the number of items displayed per page while pageNumber > 1; see fd2d1f2a49ad4f6bc2b5ed8bf3aecd65093abc65.
---
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-12490] Don't use Javascript for web UI'...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167685179
okey :)
---
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-12490] Don't use Javascript for web UI'...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166754320
LGTM except my comments about `Utils.exceptionString`
---
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-12490] Don't use Javascript for web UI'...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-167677076
LGTM
---
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-12490] Don't use Javascript for web UI'...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10441#issuecomment-166806823
**[Test build #48227 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48227/consoleFull)** for PR 10441 at commit [`89827a5`](https://github.com/apache/spark/commit/89827a505f51ffce64d03f6115e7d9d801ca7ead).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:\n * `case class AssertNotNull(`\n
---
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