[
https://issues.apache.org/jira/browse/YARN-3975?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14729904#comment-14729904
]
Jason Lowe commented on YARN-3975:
----------------------------------
Thanks for updating the patch, Mit!
Having two separate booleans for RM vs. AHS seems a bit odd and leads to weird
situations like the possibiltiy they're both true. Normally this is handled
via an enum which would make the code a bit cleaner and eliminate the potential
for weird states.
I think it would be a bit clearer if ApplicationReportBundle were named
something like FetchedAppReport or something similar.
To reduce the amount of unnecessary boilerplate associated with the new class,
I think the constructor should take the app report and the origination enum and
there should only be getters. We should be able to treat a fetched application
report like an immutable object, and I don't see a need to have setters on it.
Nit: a number of comments that were added in the patch are exactly parroting
the code and aren't adding value, e.g.:
{noformat}
//set this flag to true so that WebAppProxyServlet can understand that the
//appReport was fetched from the RM
isReportFromRM = true;
[...]
//Create a bundle of appReport and the source from where it was fetched
//so the WebAppProxyServlet has a way to know the appReport source
ApplicationReportBundle appReportBundle = new ApplicationReportBundle();
{noformat}
Rather than tack on another set of tests to an already involved test method, it
would be cleaner to have a separate test case for the new functionality.
> WebAppProxyServlet should not redirect to RM page if AHS is enabled
> -------------------------------------------------------------------
>
> Key: YARN-3975
> URL: https://issues.apache.org/jira/browse/YARN-3975
> Project: Hadoop YARN
> Issue Type: Bug
> Affects Versions: 2.7.1
> Reporter: Mit Desai
> Assignee: Mit Desai
> Attachments: YARN-3975.2.b2.patch, YARN-3975.3.patch,
> YARN-3975.4.patch, YARN-3975.5.patch, YARN-3975.6.patch
>
>
> WebAppProxyServlet should be updated to handle the case when the appreport
> doesn't have a tracking URL and the Application History Server is eanbled.
> As we would have already tried the RM and got the
> ApplicationNotFoundException we should not direct the user to the RM app page.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
