SPARK-1417: Spark on Yarn - spark UI link from resourcemanager is broken #344
Conversation
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13843/ |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13851/ |
| /** | ||
| * Return the application UI address. This does not include the scheme (http://). | ||
| */ | ||
| private[spark] def appUIAddress = publicHost + ":" + boundPort |
There was a problem hiding this comment.
hm - rather than doing this what about having appUIHostPort in addition to appUIAddress. Even though address is not an official term, I think people downstream would expect it to have a scheme (ala http://en.wikipedia.org/wiki/Uniform_resource_locator).
There was a problem hiding this comment.
I'm fine with that. That is actually what I originally had back when I put in the support for linking spark ui to yarn ui, but during the review we decided to remove it since it was only used in one other spot.
|
Merged build triggered. |
|
Merged build started. |
|
|
||
| test("verify appUIHostPort doesn't contain scheme") { | ||
| val appUIUri = new URI(sc.ui.appUIHostPort) | ||
| assert(appUIUri.getScheme().startsWith("http") == false) |
There was a problem hiding this comment.
nit: assert(!appUIUri.getScheme().startsWith("http")) (similarly in L33)
|
Merged build triggered. |
|
Merged build started. |
|
Thanks, |
|
Thanks for the review, updated it to remove extra newline and simplify the assert. (and I accidentally hit the close button instead of the comment button). |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13859/ |
| * Return the application UI host:port. This does not include the scheme (http://). | ||
| */ | ||
| private[spark] def appUIHostPort = publicHost + ":" + boundPort | ||
|
|
||
| private[spark] def appUIAddress = "http://" + publicHost + ":" + boundPort |
There was a problem hiding this comment.
Pedantic - but this one could just use appUIHostPort - tiny thing, don't let it block you from merging this.
private[spark] def appUIAddress = s"http://$appUIHostPort"
There was a problem hiding this comment.
thanks for catching that. I had meant to do that, updating.
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13857/ |
|
Merged build triggered. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13895/ |
|
Is jenkins not picking up the latest changes? The test it says is failing works fine for me. |
|
It seems that URI requires a scheme in the parameter, so |
|
Oh its because jenkins is picking up an ip instead of when I run it locally it picks up the host. Ok, let me fix that test. |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. |
|
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/13901/ |
|
Jenkins, test this please |
|
Merged build triggered. |
|
Merged build started. |
|
Merged build finished. All automated tests passed. |
|
All automated tests passed. |
|
Merging this into master. |
|
I pulled this into branch-1.0. |
Author: Thomas Graves <[email protected]> Closes #344 from tgravescs/SPARK-1417 and squashes the following commits: c450b5f [Thomas Graves] fix test e1c1d7e [Thomas Graves] add missing $ to appUIAddress e982ddb [Thomas Graves] use appUIHostPort in appUIAddress 0803ec2 [Thomas Graves] Review comment updates - remove extra newline, simplify assert in test 658a8ec [Thomas Graves] Add a appUIHostPort routine 0614208 [Thomas Graves] Fix test 2a6b1b7 [Thomas Graves] SPARK-1417: Spark on Yarn - spark UI link from resourcemanager is broken
|
Cool! Thanks @tgravescs |
Author: Thomas Graves <[email protected]> Closes apache#344 from tgravescs/SPARK-1417 and squashes the following commits: c450b5f [Thomas Graves] fix test e1c1d7e [Thomas Graves] add missing $ to appUIAddress e982ddb [Thomas Graves] use appUIHostPort in appUIAddress 0803ec2 [Thomas Graves] Review comment updates - remove extra newline, simplify assert in test 658a8ec [Thomas Graves] Add a appUIHostPort routine 0614208 [Thomas Graves] Fix test 2a6b1b7 [Thomas Graves] SPARK-1417: Spark on Yarn - spark UI link from resourcemanager is broken
* Fix sbt build. - Remove extraneous Feign dependency that we no longer use in submission v2. - Exclude Jackson from various modules to ensure every Jackson module is forced to 2.6.5. - Fix a linter error only caught by sbt. - Add Kubernetes modules to various parts of the SBT infrastructure * Actually remove feign * Actually exclude Jackson from kubernetes client.
* Fix sbt build. - Remove extraneous Feign dependency that we no longer use in submission v2. - Exclude Jackson from various modules to ensure every Jackson module is forced to 2.6.5. - Fix a linter error only caught by sbt. - Add Kubernetes modules to various parts of the SBT infrastructure * Actually remove feign * Actually exclude Jackson from kubernetes client.
…origin transformexpression with origin
Add domain_id of public clouds into secrets.yaml
No description provided.