SPARK-5087. [YARN] Merge yarn.Client and yarn.ClientBase

[sryza]

No description provided.

[sryza]

@andrewor14 @tgravescs

[SparkQA]

Test build #25048 has started for PR 3896 at commit 930df8a.

• This patch merges cleanly.

[SparkQA]

Test build #25048 has finished for PR 3896 at commit 930df8a.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25048/
Test PASSed.

[tgravescs]

what is this import for?

[andrewor14]

for helper methods defined in object Client I believe

[sryza]

Yup. We used to import ClientBase._

[andrewor14]

@sryza looks pretty straightforward to me. I notice there are a couple methods that aren't fully private only because they're exposed for testing. If possible, could we make these private[yarn] at the least? Also, have you had a chance to test this on a YARN cluster?

[sryza]

Thanks for taking a look Andrew and Tom. I tested it with both yarn-client and yarn-cluster mode and things look fine. I originally tried making getApplicationReport private[yarn], but discovered that it's needed by YarnClientSchedulerBackend.

Posting a new patch with the suggested changes.

[SparkQA]

Test build #25167 has started for PR 3896 at commit 434ad3a.

• This patch merges cleanly.

[SparkQA]

Test build #25167 has finished for PR 3896 at commit 434ad3a.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25167/
Test FAILed.

[andrewor14]

retest this please

[SparkQA]

Test build #25185 has started for PR 3896 at commit 434ad3a.

• This patch merges cleanly.

[SparkQA]

Test build #25185 has finished for PR 3896 at commit 434ad3a.

• This patch fails Spark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25185/
Test FAILed.

[sryza]

The failures are legit. Working on an update.

[sryza]

It looks like copyFileToRemote was a class method so that it could be mocked for a test (Mockito can't mock static methods). There might be a more elegant way to write the test that doesn't require this, but I think that's probably out of the scope of this change, so I'm going to put it back for now.

[SparkQA]

Test build #25191 has started for PR 3896 at commit 65611d0.

• This patch merges cleanly.

[SparkQA]

Test build #25191 has finished for PR 3896 at commit 65611d0.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25191/
Test PASSed.

[andrewor14]

Ok LGTM I'm merging this into master thanks