[
https://issues.apache.org/jira/browse/YARN-955?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13816872#comment-13816872
]
Zhijie Shen commented on YARN-955:
----------------------------------
1. It's not necessary, and shouldn't be public.
{code}
+ public ApplicationAttemptReport getApplicationAttempt(
+ ApplicationAttemptId appAttemptId) throws IOException {
+ return history.getApplicationAttempt(appAttemptId);
+ }
{code}
2. It's not a recommended way to construct pb instances. Please use
XXXX.newInstance() for all the responses here.
{code}
+ GetApplicationAttemptReportResponse response = Records
+ .newRecord(GetApplicationAttemptReportResponse.class);
{code}
3. Why the reference of the implementation is used? Should we use
ApplicationHistoryManager, the interface, instead?
{code}
+ ApplicationHistoryManagerImpl historyService;
{code}
4. In TestApplicationHistoryClientService, please write multiple instances, and
test getXXXXs methods as well.
5. Add \@VisibleForTesting
{code}
+ @Private
+ public ApplicationHistoryClientService getClientService() {
+ return this.ahsClientService;
+ }
{code}
6. Unwrap the method and put it directly in ApplicationHistoryServer.main
{code}
+ static ApplicationHistoryServer launchAppHistoryServer(String[] args) {
{code}
> [YARN-321] Implementation of ApplicationHistoryProtocol
> -------------------------------------------------------
>
> Key: YARN-955
> URL: https://issues.apache.org/jira/browse/YARN-955
> Project: Hadoop YARN
> Issue Type: Sub-task
> Reporter: Vinod Kumar Vavilapalli
> Assignee: Mayank Bansal
> Attachments: YARN-955-1.patch, YARN-955-2.patch, YARN-955-3.patch
>
>
--
This message was sent by Atlassian JIRA
(v6.1#6144)
