Mailing-List: contact yarn-issues-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Date: Mon, 17 Oct 2016 21:40:58 +0000 (UTC)
From: "Wangda Tan (JIRA)" <jira@apache.org>
To: yarn-issues@hadoop.apache.org
Message-ID: <JIRA.12999217.1471958678000.9696.1476740458362@Atlassian.JIRA>
In-Reply-To: <JIRA.12999217.1471958678000@Atlassian.JIRA>
References: <JIRA.12999217.1471958678000@Atlassian.JIRA> <JIRA.12999217.1471958678971@arcas>
Subject: [jira] [Comment Edited] (YARN-5552) Add Builder methods for common
 yarn API records
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
archived-at: Mon, 17 Oct 2016 21:41:00 -0000


    [ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15583572#comment-15583572 ] 

Wangda Tan edited comment on YARN-5552 at 10/17/16 9:40 PM:
------------------------------------------------------------

Thanks [~Tao Jie].

Few minor comments from my side: 

1) New added methods of AllocateResponse should be all @Private and @Unstable, we should not mark original write-methods of AllocateResponse to @Public at the beginning, the reason is AllocateResponse should be read-only by YARN app, no YARN app should set these fields. YARN services should take care of settings these fields of AllocateResponse.

2) Can we merge common sanity check logic of ContainerRequestBuilder#build and ContainerRequest#constructor?

3) For all new added write methods of builder, it's better to add a javadocs link-to reference original method.


was (Author: leftnoteasy):
Thanks [~Tao Jie].

Few minor comments from my side: 

1) New added methods of AllocateResponse should be all @Private and @Unstable, we should not mark original write-methods of AllocateResponse to @Public at the beginning, the reason is AllocateResponse should be read-only, no YARN app should set these fields.

2) Can we merge common sanity check logic of ContainerRequestBuilder#build and ContainerRequest#constructor?

3) For all new added write methods of builder, it's better to add a javadocs link-to reference original method.

> Add Builder methods for common yarn API records
> -----------------------------------------------
>
>                 Key: YARN-5552
>                 URL: https://issues.apache.org/jira/browse/YARN-5552
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Arun Suresh
>            Assignee: Tao Jie
>         Attachments: YARN-5552.000.patch, YARN-5552.001.patch, YARN-5552.002.patch, YARN-5552.003.patch, YARN-5552.004.patch, YARN-5552.005.patch
>
>
> Currently yarn API records such as ResourceRequest, AllocateRequest/Respone as well as AMRMClient.ContainerRequest have multiple constructors / newInstance methods. This makes it very difficult to add new fields to these records.
> It would probably be better if we had Builder classes for many of these records, which would make evolution of these records a bit easier.
> (suggested by [~kasha])


--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

---------------------------------------------------------------------
To unsubscribe, e-mail: yarn-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: yarn-issues-help@hadoop.apache.org