Fixes Spark REST fetcher for client mode applications

[shkhrgpt]

As mentioned in the issue, #175, the current REST API fetcher fails to parse client mode applications because there is no attemptId in REST response of ApplicationInfo.

In this change, we are modifying the fetcher to not consider attemptId in REST urls when attemptId is not returned by ApplicationInfo response, as it's mentioned in this Spark documentation.

With this change, the fetcher will be able to process both Client and Cluster mode applications.

@akshayrai can you please have a look.

Thank you.

[shkhrgpt]

@rayortigas , @paulbramsen , @shankar37 can you please review this change. This is a critical and active issue.
Thank you.

[shankar37]

@rayortigas can you take a look as well ?

[rayortigas]

Code looks fine. Just wanted to add a couple of comments:

1. I'm well aware that the API can serve requests without an attempt ID. However, the old Spark fetcher didn't appear to support yarn-client, considering that it was hardcoding _1 when fetching the Spark event logs. See https://github.com/linkedin/dr-elephant/pull/162/files#diff-24b9ededac05bd3a6096311b696d0830L240 For my education, I'd like to know whether a) I introduced a regression and missed something, or b) if this also wasn't working before Rewrite Spark fetcher/heuristics. #162, and we only expected to support yarn-cluster mode.

2. I feel the corresponding unit test should be updated. It's probably way more code than the fix provided here, but it'll reinforce/document expectations about supporting both yarn-client and yarn-cluster modes, which the previous unit test didn't appear to do https://github.com/linkedin/dr-elephant/pull/162/files#diff-25f92aef2adc15b2196f1f12b5344549L39

[shkhrgpt]

@rayortigas: I am not very familiar with the old Spark fetcher, so I am not sure whether or not it supported yarn-client. Maybe @akshayrai or @shankar37 can provide some input here. We submit Spark applications both in cluster and client mode, and Dr Elephant wasn't able to fetch client mode applications. Later I also found an issue (#175), where other people are also trying to use Dr Elephant to analyze client mode applications and it's failing.

As per your second concern, I have added a unit test to test this change. Please take a look.

Thank you.

[rayortigas]

@shkhrgpt LGTM. Thanks for adding the unit test!

Yeah, the other question was primarily for @akshayrai and @shankar37.

[shkhrgpt]

@akshayrai and @shankar37 Would you like me to make any more changes in order to merge to this PR? There is still a critical and active issue which requires this fix.
Thank you.

[akshayrai]

+1 LGTM