You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by lw-lin <gi...@git.apache.org> on 2016/03/19 13:47:05 UTC
[GitHub] spark pull request: [SPARK-14025][STREAMING][WEBUI] Fix streaming ...
GitHub user lw-lin opened a pull request:
https://github.com/apache/spark/pull/11845
[SPARK-14025][STREAMING][WEBUI] Fix streaming job descriptions on the event line
## What changes were proposed in this pull request?
Removed the extra `<a href=...>...</a>` for each streaming job's description on the event line.
### [Before]
### [After]
## How was this patch tested?
test suits, manual checks (see screenshots above)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/lw-lin/spark description-event-line
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/11845.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 #11845
----
commit 0f32845edb58be9b5a935136d0cd3460438bb9f2
Author: proflin <pr...@gmail.com>
Date: 2016-03-19T09:35:55Z
Add job/stage descriptionPlain for event-line
----
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-198714597
Maybe let's summarize a little bit:
- job/stage descriptions are used at 2 places: the `Event Timeline` and the text tables (`CompletedJobs`/`CompletedStages`)
- job/stage descriptions were intended to contains only texts at first, so everything's well
- https://github.com/apache/spark/pull/8791 introduced HTML in job/stage description for streaming jobs
- the text tables works well, because it renders HTML links directly, without escaping the description
- the event time line escapes the description intentionally, and then displays the description, leading to the plain `<a href></a>` thing
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199750468
It failed on a line you modified so it is due to this change. The error message explains what you need to change.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14025][STREAMING][WEBUI] Fix streaming ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199866576
**[Test build #53770 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53770/consoleFull)** for PR 11845 at commit [`ebe1182`](https://github.com/apache/spark/commit/ebe11822ef3113258f8f2ca6723a36bbdc9d20fc).
* 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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-200573435
Thank you all for your kind review & comments, @srowen @andrewor14 @zsxwing !
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199680181
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/53755/
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-198715624
My question is, why escape it? the library will accept and render HTML and that is desirable here 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: [SPARK-14025][STREAMING][WEBUI] Fix streaming ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/11845
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199402501
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-198695846
@andrewor14 @zsxwing would you mind taking a look at this when you have time? 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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199583350
@andrewor14 thank you for the informative review. Will soon update this PR accordingly.
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-198708658
Besides, the blue/green bar in the event line itself is a clickable, linking to the specific job page. The `<a href>` thing is superfluous, let's figure out how to remove 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: [SPARK-14025][STREAMING][WEBUI] Fix streaming ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199680174
**[Test build #53755 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53755/consoleFull)** for PR 11845 at commit [`22ac2d9`](https://github.com/apache/spark/commit/22ac2d97a117108924c4deec1a132f8fc70cbc0c).
* This patch **fails Scala style 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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199402457
**[Test build #53688 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53688/consoleFull)** for PR 11845 at commit [`0f32845`](https://github.com/apache/spark/commit/0f32845edb58be9b5a935136d0cd3460438bb9f2).
* 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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199867357
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199402824
@lw-lin thanks for summarizing the issue. Yes, we don't want to process arbitrary HTML so we don't have to worry about potential XSS threats. However, I agree with @srowen that the approach you chose here seems kind of arbitrary. Instead, a better place to do this might be in `UIUtils.makeDescription`. There we already parse the description for anchor tags, so we can just pass in an additional flag there to optionally remove the tags.
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-200565179
LGTM. Merging to master. Thanks, @lw-lin
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-200168135
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199867366
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/53770/
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-198695774
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14025][STREAMING][WEBUI] Fix streaming ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-198709371
Ah I thought this was a tooltip, where HTML can't render, but it's not. I wonder, is the problem just that this is rendered as text and not HTML? if it's controlled by the UI, it seems safe to just let it render as HTML if possible?
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199396690
**[Test build #53688 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53688/consoleFull)** for PR 11845 at commit [`0f32845`](https://github.com/apache/spark/commit/0f32845edb58be9b5a935136d0cd3460438bb9f2).
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-198702731
I agree it's a problem, but this seems like a hacky way to band-aid it, with a second version of the description and a flag passed around. Is the problem not just that this is HTML, but not being interpreted as such?
Is it necessary that this description have HTML to begin with?
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-198718851
Maybe @andrewor14 will explain better :)
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199811621
**[Test build #53770 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53770/consoleFull)** for PR 11845 at commit [`ebe1182`](https://github.com/apache/spark/commit/ebe11822ef3113258f8f2ca6723a36bbdc9d20fc).
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-200168137
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/53869/
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11845#discussion_r57082998
--- Diff: core/src/main/scala/org/apache/spark/ui/UIUtils.scala ---
@@ -417,7 +417,7 @@ private[spark] object UIUtils extends Logging {
* attempts to embed links outside Spark UI, or other tags like <script> will cause in the whole
* description to be treated as plain text.
*/
- def makeDescription(desc: String, basePathUri: String): NodeSeq = {
+ def makeDescription(desc: String, basePathUri: String, plainText: Boolean = false): NodeSeq = {
--- End diff --
can you add a comment in the java doc to explain what `plainText` means
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199680178
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-200127975
**[Test build #53869 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53869/consoleFull)** for PR 11845 at commit [`0761e5f`](https://github.com/apache/spark/commit/0761e5f5485fff1f99c0b8b7dc6bfe8f8f5a0b82).
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199679910
**[Test build #53755 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53755/consoleFull)** for PR 11845 at commit [`22ac2d9`](https://github.com/apache/spark/commit/22ac2d97a117108924c4deec1a132f8fc70cbc0c).
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-198707142
@srowen thanks for looking at this!
I believe job descriptions were intended to contains only plain texts at first, but HTMLs were introduced in for streaming jobs by https://github.com/apache/spark/pull/8791. Hyperlinks in the `CompletedJobs`/`CompletedStages` tables are proven quite useful, so I think may we'll need one more version of the job description? Any thoughts on how to make this elegant would be appreciated :)
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-198718502
Ah, I guess the reason is:
- for the blue box in the event timeline, itself is clickable both for streaming/non-streaming jobs, so it's unnecessary to contain any HTML like `<a href></a>`
- for the black tooltip, it is designed to contain only plain texts. The tooltip would just move with the cursor, so even it does contain a link, no one can manage to click on it.
The original tooltip library does accept and render HTML, but I guess we Spark developers decide we only need plain texts here (see the above 2 points), so we'll escape things to prevent from malicious codes. However then HTMLs(`<a href></a>`) slipped in along with PR#8791 :)
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199402507
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/53688/
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199727209
Updated with new commits.
Tests passed on my local machine; I wonder why scalastyle checks failed (this PR doesn't touch this code snippet):
>
Scalastyle checks failed at following occurrences:
[error] /home/jenkins/workspace/SparkPullRequestBuilder/core/src/test/scala/org/apache/spark/ui/UIUtilsSuite.scala:64:0: Whitespace at end of line
[error] \(core/test:scalastyle\) errors exist
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-200066336
Looks pretty good. @zsxwing can you have a look?
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/11845#discussion_r57082964
--- Diff: core/src/main/scala/org/apache/spark/ui/UIUtils.scala ---
@@ -445,22 +445,36 @@ private[spark] object UIUtils extends Logging {
"Links in job descriptions must be root-relative:\n" + allLinks.mkString("\n\t"))
}
- // Prepend the relative links with basePathUri
- val rule = new RewriteRule() {
- override def transform(n: Node): Seq[Node] = {
- n match {
- case e: Elem if e \ "@href" nonEmpty =>
- val relativePath = e.attribute("href").get.toString
- val fullUri = s"${basePathUri.stripSuffix("/")}/${relativePath.stripPrefix("/")}"
- e % Attribute(null, "href", fullUri, Null)
- case _ => n
+ val rule = if (plainText) {
--- End diff --
style:
```
val rule =
if (plainText) {
...
} else {
...
}
```
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-200167891
**[Test build #53869 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/53869/consoleFull)** for PR 11845 at commit [`0761e5f`](https://github.com/apache/spark/commit/0761e5f5485fff1f99c0b8b7dc6bfe8f8f5a0b82).
* 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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-198710234
Actually we've intentionally escaped the description for the event line, so that it will be rendered as plain texts; please see https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/ui/jobs/AllJobsPage.scala#L85.
So I think it was TD's intention to add hyperlinks to the text tables(where descriptions are not escaped), but he hadn't noticed that the same description would be escaped and used in the tooltip too.
---
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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on a diff in the pull request:
https://github.com/apache/spark/pull/11845#discussion_r57100182
--- Diff: core/src/main/scala/org/apache/spark/ui/UIUtils.scala ---
@@ -445,22 +445,36 @@ private[spark] object UIUtils extends Logging {
"Links in job descriptions must be root-relative:\n" + allLinks.mkString("\n\t"))
}
- // Prepend the relative links with basePathUri
- val rule = new RewriteRule() {
- override def transform(n: Node): Seq[Node] = {
- n match {
- case e: Elem if e \ "@href" nonEmpty =>
- val relativePath = e.attribute("href").get.toString
- val fullUri = s"${basePathUri.stripSuffix("/")}/${relativePath.stripPrefix("/")}"
- e % Attribute(null, "href", fullUri, Null)
- case _ => n
+ val rule = if (plainText) {
--- End diff --
@andrewor14 sure; updated with the new commit, 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-14025][STREAMING][WEBUI] Fix streaming ...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/11845#issuecomment-199396154
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: [SPARK-14025][STREAMING][WEBUI] Fix streaming ...
Posted by lw-lin <gi...@git.apache.org>.
Github user lw-lin commented on a diff in the pull request:
https://github.com/apache/spark/pull/11845#discussion_r57100194
--- Diff: core/src/main/scala/org/apache/spark/ui/UIUtils.scala ---
@@ -417,7 +417,7 @@ private[spark] object UIUtils extends Logging {
* attempts to embed links outside Spark UI, or other tags like <script> will cause in the whole
* description to be treated as plain text.
*/
- def makeDescription(desc: String, basePathUri: String): NodeSeq = {
+ def makeDescription(desc: String, basePathUri: String, plainText: Boolean = false): NodeSeq = {
--- End diff --
@andrewor14 sure; updated with the new commit, 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