You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by mindprince <gi...@git.apache.org> on 2015/12/07 23:33:50 UTC
[GitHub] spark pull request: [SPARK-12186] [WEB UI] Send the complete reque...
GitHub user mindprince opened a pull request:
https://github.com/apache/spark/pull/10180
[SPARK-12186] [WEB UI] Send the complete request URI including the query string when redirecting.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mindprince/spark SPARK-12186
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/10180.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 #10180
----
commit 7625c88e15cc4b791759be40ca560b7b98a8e2cc
Author: Rohit Agarwal <ro...@qubole.com>
Date: 2015-12-07T22:32:19Z
SPARK-12186: Send the complete request URI including the query string when redirecting.
----
---
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-12186] [WEB UI] Send the complete reque...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-164543275
LGTM once you address the comments.
---
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-12186] [WEB UI] Send the complete reque...
Posted by mindprince <gi...@git.apache.org>.
Github user mindprince commented on a diff in the pull request:
https://github.com/apache/spark/pull/10180#discussion_r47817405
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -103,7 +103,11 @@ class HistoryServer(
// Note we don't use the UI retrieved from the cache; the cache loader above will register
// the app's UI, and all we need to do is redirect the user to the same URI that was
// requested, and the proper data should be served at that point.
--- End diff --
Done.
---
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-12186] [WEB UI] Send the complete reque...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/10180
---
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-12186] [WEB UI] Send the complete reque...
Posted by mindprince <gi...@git.apache.org>.
Github user mindprince commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-163027364
cc - @vanzin
---
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-12186] [WEB UI] Send the complete reque...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-163795503
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-12186] [WEB UI] Send the complete reque...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-163818607
**[Test build #47555 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47555/consoleFull)** for PR 10180 at commit [`7625c88`](https://github.com/apache/spark/commit/7625c88e15cc4b791759be40ca560b7b98a8e2cc).
* 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-12186] [WEB UI] Send the complete reque...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-165244325
**[Test build #47853 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47853/consoleFull)** for PR 10180 at commit [`780ee62`](https://github.com/apache/spark/commit/780ee624b23703b11c4c90b74b80af8202abd21e).
---
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-12186] [WEB UI] Send the complete reque...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-165211776
**[Test build #47839 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47839/consoleFull)** for PR 10180 at commit [`780ee62`](https://github.com/apache/spark/commit/780ee624b23703b11c4c90b74b80af8202abd21e).
---
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-12186] [WEB UI] Send the complete reque...
Posted by mindprince <gi...@git.apache.org>.
Github user mindprince commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-164333244
Can someone review this or if it seems okay, merge 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-12186] [WEB UI] Send the complete reque...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-165233273
**[Test build #47839 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47839/consoleFull)** for PR 10180 at commit [`780ee62`](https://github.com/apache/spark/commit/780ee624b23703b11c4c90b74b80af8202abd21e).
* 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-12186] [WEB UI] Send the complete reque...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-165233362
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47839/
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-12186] [WEB UI] Send the complete reque...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-165284298
**[Test build #47853 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47853/consoleFull)** for PR 10180 at commit [`780ee62`](https://github.com/apache/spark/commit/780ee624b23703b11c4c90b74b80af8202abd21e).
* 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-12186] [WEB UI] Send the complete reque...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-163818726
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47555/
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-12186] [WEB UI] Send the complete reque...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-165284458
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47853/
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-12186] [WEB UI] Send the complete reque...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/10180#discussion_r47549398
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -103,7 +103,11 @@ class HistoryServer(
// Note we don't use the UI retrieved from the cache; the cache loader above will register
// the app's UI, and all we need to do is redirect the user to the same URI that was
// requested, and the proper data should be served at that point.
- res.sendRedirect(res.encodeRedirectURL(req.getRequestURI()))
+ if (req.getQueryString == null) {
--- End diff --
or even
```
val requestURI = req.getRequestURI + req.getQueryString.map("?" + _).getOrElse("")
```
---
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-12186] [WEB UI] Send the complete reque...
Posted by mindprince <gi...@git.apache.org>.
Github user mindprince commented on a diff in the pull request:
https://github.com/apache/spark/pull/10180#discussion_r47733082
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -103,7 +103,11 @@ class HistoryServer(
// Note we don't use the UI retrieved from the cache; the cache loader above will register
// the app's UI, and all we need to do is redirect the user to the same URI that was
// requested, and the proper data should be served at that point.
- res.sendRedirect(res.encodeRedirectURL(req.getRequestURI()))
+ if (req.getQueryString == null) {
--- End diff --
@andrewor14 Can't use `getOrElse`:
```
value getOrElse is not a member of scala.collection.immutable.IndexedSeq[String]
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12186] [WEB UI] Send the complete reque...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-165240392
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-12186] [WEB UI] Send the complete reque...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-163818724
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-12186] [WEB UI] Send the complete reque...
Posted by mindprince <gi...@git.apache.org>.
Github user mindprince commented on a diff in the pull request:
https://github.com/apache/spark/pull/10180#discussion_r47735900
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -103,7 +103,11 @@ class HistoryServer(
// Note we don't use the UI retrieved from the cache; the cache loader above will register
// the app's UI, and all we need to do is redirect the user to the same URI that was
// requested, and the proper data should be served at that point.
- res.sendRedirect(res.encodeRedirectURL(req.getRequestURI()))
+ if (req.getQueryString == null) {
--- End diff --
It is.
---
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-12186] [WEB UI] Send the complete reque...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-165322636
Merged into master 1.6
---
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-12186] [WEB UI] Send the complete reque...
Posted by srowen <gi...@git.apache.org>.
Github user srowen commented on a diff in the pull request:
https://github.com/apache/spark/pull/10180#discussion_r47508088
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -103,7 +103,11 @@ class HistoryServer(
// Note we don't use the UI retrieved from the cache; the cache loader above will register
// the app's UI, and all we need to do is redirect the user to the same URI that was
// requested, and the proper data should be served at that point.
- res.sendRedirect(res.encodeRedirectURL(req.getRequestURI()))
+ if (req.getQueryString == null) {
--- End diff --
I think this is fine, but maybe slightly less repetitious like ...
```
val requestURI = req.getRequestURI + (if (req.getQueryString == null) "" else "?" + req.getQueryString)
res.sendRedirect(res.encodeRedirectURL(requestURI))
---
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-12186] [WEB UI] Send the complete reque...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-165284457
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-12186] [WEB UI] Send the complete reque...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/10180#discussion_r47549671
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -103,7 +103,11 @@ class HistoryServer(
// Note we don't use the UI retrieved from the cache; the cache loader above will register
// the app's UI, and all we need to do is redirect the user to the same URI that was
// requested, and the proper data should be served at that point.
--- End diff --
can you expand on this comment, summarizing SPARK-12186 in 1 sentence?
---
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-12186] [WEB UI] Send the complete reque...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/10180#discussion_r47734062
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -103,7 +103,11 @@ class HistoryServer(
// Note we don't use the UI retrieved from the cache; the cache loader above will register
// the app's UI, and all we need to do is redirect the user to the same URI that was
// requested, and the proper data should be served at that point.
- res.sendRedirect(res.encodeRedirectURL(req.getRequestURI()))
+ if (req.getQueryString == null) {
--- End diff --
what? Isn't `req.getQueryString` a string?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12186] [WEB UI] Send the complete reque...
Posted by mindprince <gi...@git.apache.org>.
Github user mindprince commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-163795104
cc - @srowen, @andrewor14 Can you please ask Jenkins to run the tests. 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-12186] [WEB UI] Send the complete reque...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/10180#discussion_r47740996
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -103,7 +103,11 @@ class HistoryServer(
// Note we don't use the UI retrieved from the cache; the cache loader above will register
// the app's UI, and all we need to do is redirect the user to the same URI that was
// requested, and the proper data should be served at that point.
- res.sendRedirect(res.encodeRedirectURL(req.getRequestURI()))
+ if (req.getQueryString == null) {
--- End diff --
oh sorry, I meant
```
val requestURI = req.getRequestURI + Option(req.getQueryString).map("?" + _).getOrElse("")
```
---
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-12186] [WEB UI] Send the complete reque...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-165233361
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-12186] [WEB UI] Send the complete reque...
Posted by mindprince <gi...@git.apache.org>.
Github user mindprince commented on a diff in the pull request:
https://github.com/apache/spark/pull/10180#discussion_r47733873
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -103,7 +103,11 @@ class HistoryServer(
// Note we don't use the UI retrieved from the cache; the cache loader above will register
// the app's UI, and all we need to do is redirect the user to the same URI that was
// requested, and the proper data should be served at that point.
- res.sendRedirect(res.encodeRedirectURL(req.getRequestURI()))
+ if (req.getQueryString == null) {
--- End diff --
@srowen I think having an `if` statement like that makes it a little harder to read. How about the following, it has lesser repetition.
```
val requestURI = if (req.getQueryString == null) {
req.getRequestURI
} else {
req.getRequestURI + "?" + req.getQueryString
}
res.sendRedirect(res.encodeRedirectURL(requestURI))
```
---
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-12186] [WEB UI] Send the complete reque...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-162691068
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-12186] [WEB UI] Send the complete reque...
Posted by mindprince <gi...@git.apache.org>.
Github user mindprince commented on a diff in the pull request:
https://github.com/apache/spark/pull/10180#discussion_r47817475
--- Diff: core/src/main/scala/org/apache/spark/deploy/history/HistoryServer.scala ---
@@ -103,7 +103,11 @@ class HistoryServer(
// Note we don't use the UI retrieved from the cache; the cache loader above will register
// the app's UI, and all we need to do is redirect the user to the same URI that was
// requested, and the proper data should be served at that point.
- res.sendRedirect(res.encodeRedirectURL(req.getRequestURI()))
+ if (req.getQueryString == null) {
--- End diff --
Updated.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-12186] [WEB UI] Send the complete reque...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/10180#issuecomment-163796524
**[Test build #47555 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47555/consoleFull)** for PR 10180 at commit [`7625c88`](https://github.com/apache/spark/commit/7625c88e15cc4b791759be40ca560b7b98a8e2cc).
---
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