[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15658297#comment-15658297 ] Hudson commented on YARN-5552: -- SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #10824 (See [https://builds.apache.org/job/Hadoop-trunk-Commit/10824/]) YARN-5552. Add Builder methods for common yarn API records. (Tao Jie via (wangda: rev ede1a473f5061cf40f6affc1c8c30a645c1fef6c) * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/impl/pb/AllocateResponsePBImpl.java * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/common/fica/FiCaSchedulerApp.java * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-resourcemanager/src/main/java/org/apache/hadoop/yarn/server/resourcemanager/scheduler/AppSchedulingInfo.java * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/impl/AMRMClientImpl.java * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/records/ResourceRequest.java * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client/src/main/java/org/apache/hadoop/yarn/client/api/AMRMClient.java * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/AllocateRequest.java * (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/api/protocolrecords/AllocateResponse.java > 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 > Fix For: 2.9.0 > > 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, YARN-5552.006.patch, YARN-5552.007.patch, > YARN-5552.008.patch, YARN-5552.009.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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15653301#comment-15653301 ] Karthik Kambatla commented on YARN-5552: Eye-balled the changes and they look good to me. Defer to [~leftnoteasy] for a more thorough review. > 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, YARN-5552.006.patch, YARN-5552.007.patch, > YARN-5552.008.patch, YARN-5552.009.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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15633772#comment-15633772 ] Wangda Tan commented on YARN-5552: -- Patch looks good, will wait till next Monday before committing. Any other suggestions? [~asuresh] / [~kasha]. > 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, YARN-5552.006.patch, YARN-5552.007.patch, > YARN-5552.008.patch, YARN-5552.009.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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15631581#comment-15631581
]
Hadoop QA commented on YARN-5552:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
21s{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
9s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 7m
9s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
23s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
40s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m
4s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
59s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
29s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m
23s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
10s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
45s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
31s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 2m
31s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
38s{color} | {color:green} hadoop-yarn-project/hadoop-yarn: The patch generated
0 new + 109 unchanged - 8 fixed = 109 total (was 117) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m
0s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
55s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
52s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m
19s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m
25s{color} | {color:green} hadoop-yarn-api in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
18s{color} | {color:green} hadoop-yarn-common in the patch passed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 35m 52s{color}
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 16m
24s{color} | {color:green} hadoop-yarn-client in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
22s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 95m 20s{color} |
{color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests |
hadoop.yarn.server.resourcemanager.applicationsmanager.TestAMRestart |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:9560f25 |
| JIRA Issue | YARN-5552 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12836714/YARN-5552.009.patch |
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit findbugs checkstyle |
| uname | Linux f9d578bde5d4 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed
Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.s
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15630820#comment-15630820 ] Wangda Tan commented on YARN-5552: -- [~Tao Jie] could you check Java docs warnings as well? Our policy is make sure no new java docs warnings added for committed patch. Thanks, > 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, YARN-5552.006.patch, YARN-5552.007.patch, > YARN-5552.008.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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15628174#comment-15628174
]
Hadoop QA commented on YARN-5552:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
14s{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
53s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
52s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
28s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
42s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m
8s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
56s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
38s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m
23s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
10s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
42s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
27s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 2m
27s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
38s{color} | {color:green} hadoop-yarn-project/hadoop-yarn: The patch generated
0 new + 110 unchanged - 8 fixed = 110 total (was 118) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m
1s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
54s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
54s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
17s{color} | {color:red} hadoop-yarn-project_hadoop-yarn_hadoop-yarn-api
generated 56 new + 123 unchanged - 0 fixed = 179 total (was 123) {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m
25s{color} | {color:green} hadoop-yarn-api in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
16s{color} | {color:green} hadoop-yarn-common in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 35m
24s{color} | {color:green} hadoop-yarn-server-resourcemanager in the patch
passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 16m
3s{color} | {color:green} hadoop-yarn-client in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
19s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 94m 56s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:9560f25 |
| JIRA Issue | YARN-5552 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12836513/YARN-5552.008.patch |
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit findbugs checkstyle |
| uname | Linux 16a865124b74 3.13.0-93-generic #140-Ubuntu SMP Mon Jul 18
21:21:05 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | trunk / cb5c
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15625948#comment-15625948
]
Wangda Tan commented on YARN-5552:
--
Thanks [~Tao Jie], generally looks good.
Could you also mark {{AllocateResponse#newBuilder}} as private and unstable?
And also, it seems javadocs failure is related to the patch, could you double
check?
> 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, YARN-5552.006.patch, YARN-5552.007.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-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15625607#comment-15625607
]
Hadoop QA commented on YARN-5552:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
21s{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
11s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
59s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
19s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
40s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m
1s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
59s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
34s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m
23s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
10s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
43s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
24s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 2m
24s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
40s{color} | {color:green} hadoop-yarn-project/hadoop-yarn: The patch generated
0 new + 109 unchanged - 8 fixed = 109 total (was 117) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m
2s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
56s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s{color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 4m
9s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
18s{color} | {color:red} hadoop-yarn-api in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m
27s{color} | {color:green} hadoop-yarn-api in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
19s{color} | {color:green} hadoop-yarn-common in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 40m
28s{color} | {color:green} hadoop-yarn-server-resourcemanager in the patch
passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 16m
4s{color} | {color:green} hadoop-yarn-client in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
21s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 99m 45s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:9560f25 |
| JIRA Issue | YARN-5552 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12836347/YARN-5552.007.patch |
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit findbugs checkstyle |
| uname | Linux faeceec36a20 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed
Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | trunk / 5b577f4 |
| Default Java | 1.8.0_101 |
| findbugs | v3.0.0 |
| javadoc |
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15625276#comment-15625276
]
Hadoop QA commented on YARN-5552:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m
17s{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m
0s{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m
0s{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
9s{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
44s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
20s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
39s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m
59s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
58s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
23s{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m
24s{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m
10s{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
40s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m
18s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 2m
18s{color} | {color:green} the patch passed {color} |
| {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange}
0m 38s{color} | {color:orange} hadoop-yarn-project/hadoop-yarn: The patch
generated 39 new + 109 unchanged - 8 fixed = 148 total (was 117) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m
57s{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
55s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} whitespace {color} | {color:red} 0m
0s{color} | {color:red} The patch has 2 line(s) that end in whitespace. Use git
apply --whitespace=fix <>. Refer https://git-scm.com/docs/git-apply
{color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
51s{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
17s{color} | {color:red} hadoop-yarn-api in the patch failed. {color} |
| {color:red}-1{color} | {color:red} javadoc {color} | {color:red} 0m
14s{color} | {color:red} hadoop-yarn-project_hadoop-yarn_hadoop-yarn-client
generated 16 new + 158 unchanged - 0 fixed = 174 total (was 158) {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m
25s{color} | {color:green} hadoop-yarn-api in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m
17s{color} | {color:green} hadoop-yarn-common in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 38m
29s{color} | {color:green} hadoop-yarn-server-resourcemanager in the patch
passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 15m
56s{color} | {color:green} hadoop-yarn-client in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
19s{color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 96m 21s{color} |
{color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:9560f25 |
| JIRA Issue | YARN-5552 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12836327/YARN-5552.006.patch |
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit findbugs checkstyle |
| uname | Linux fb41009ae70
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15583572#comment-15583572 ] Wangda Tan commented on YARN-5552: -- 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=1557#comment-1557
]
Hadoop QA commented on YARN-5552:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 18s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m 0s
{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 9s
{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
57s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m 22s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
38s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m 0s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
56s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
21s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 19s
{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 10s
{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
38s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m 16s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 2m 16s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
37s {color} | {color:green} hadoop-yarn-project/hadoop-yarn: The patch
generated 0 new + 108 unchanged - 8 fixed = 108 total (was 116) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 50s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
48s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} The patch has no whitespace issues. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
50s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 13s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m 23s
{color} | {color:green} hadoop-yarn-api in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m 18s
{color} | {color:green} hadoop-yarn-common in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 39m 35s
{color} | {color:green} hadoop-yarn-server-resourcemanager in the patch passed.
{color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 16m 14s
{color} | {color:green} hadoop-yarn-client in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
17s {color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 90m 12s {color}
| {color:black} {color} |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:9560f25 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/12833366/YARN-5552.005.patch |
| JIRA Issue | YARN-5552 |
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit findbugs checkstyle |
| uname | Linux 1e64c4e288c1 3.13.0-95-generic #142-Ubuntu SMP Fri Aug 12
17:00:09 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /testptch/hadoop/patchprocess/precommit/personality/provided.sh
|
| git revision | trunk / dbe663d |
| Default Java | 1.8.0_101 |
| findbugs | v3.0.0 |
| Test Results |
https://builds.
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15575292#comment-15575292 ] Tao Jie commented on YARN-5552: --- Updated the patch according to [~leftnoteasy]'s suggestion. [~asuresh], [~kasha], [~leftnoteasy] would you mind reviewing the latest patch again? > 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15575071#comment-15575071
]
Hadoop QA commented on YARN-5552:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 18s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m 0s
{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 9s
{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
58s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m 17s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
39s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 59s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
56s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
23s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 21s
{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 9s
{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
37s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m 16s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 2m 16s
{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m 37s
{color} | {color:red} hadoop-yarn-project/hadoop-yarn: The patch generated 1
new + 116 unchanged - 8 fixed = 117 total (was 124) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 56s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
48s {color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} whitespace {color} | {color:red} 0m 0s
{color} | {color:red} The patch has 3 line(s) that end in whitespace. Use git
apply --whitespace=fix. {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
43s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 22s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m 27s
{color} | {color:green} hadoop-yarn-api in the patch passed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 2m 35s {color}
| {color:red} hadoop-yarn-common in the patch failed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 36m 37s {color}
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 16m 2s {color}
| {color:red} hadoop-yarn-client in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
19s {color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 87m 31s {color}
| {color:black} {color} |
\\
\\
|| Reason || Tests ||
| Failed junit tests | hadoop.yarn.api.TestPBImplRecords |
| |
hadoop.yarn.server.resourcemanager.scheduler.TestSchedulingWithAllocationRequestId
|
| | hadoop.yarn.server.resourcemanager.webapp.TestRMWebServicesApps |
| | hadoop.yarn.client.TestYarnApiClasses |
| | hadoop.yarn.client.api.impl.TestAMRMClient |
\\
\\
|| Subsystem || Report/Notes ||
| Docker | Image:yetus/hadoop:9560f25 |
| JIRA Patch URL |
https://issues.apache.org/jira/secure/attachment/1286/YARN-5552.004.patch |
| JIRA Issue | YARN-5552 |
| Optional Tests | asflicense compile javac javadoc mvninstall mvnsite
unit findbugs checkstyle |
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15477259#comment-15477259 ] Tao Jie commented on YARN-5552: --- Sure, thank you for your suggestion. > 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 > > > 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15470993#comment-15470993 ] Wangda Tan commented on YARN-5552: -- [~Tao Jie], bq. Storing local variables in builder here is to give a default value for fields and avoid potential NPE. It is better to set default value also in builder constructor like: Got it, but I think it's better to set these default fields within a default newInstance() method of -PBImpl. [~kasha], bq. Deprecating the old API sounds reasonable. That way, we could stop adding new fields to it. Agree. [~Tao Jie], are you agree with it? > 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 > > > 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15469780#comment-15469780
]
Tao Jie commented on YARN-5552:
---
Thank you for your comment,[~leftnoteasy]!
Storing local variables in builder here is to give a default value for fields
and avoid potential NPE. It is better to set default value also in builder
constructor like:
{quote}
private ResourceRequestBuilder() {
ResourceRequest request = Records.newRecord(ResourceRequest.class);
request.setResourceName(ResourceRequest.ANY);
request.setPriority(Priority.newInstance(0));
...
}
{quote}
> 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
>
>
> 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-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15469779#comment-15469779
]
Tao Jie commented on YARN-5552:
---
Thank you for your comment,[~leftnoteasy]!
Storing local variables in builder here is to give a default value for fields
and avoid potential NPE. It is better to set default value also in builder
constructor like:
{quote}
private ResourceRequestBuilder() {
ResourceRequest request = Records.newRecord(ResourceRequest.class);
request.setResourceName(ResourceRequest.ANY);
request.setPriority(Priority.newInstance(0));
...
}
{quote}
> 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
>
>
> 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-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15469615#comment-15469615 ] Karthik Kambatla commented on YARN-5552: Deprecating the old API sounds reasonable. That way, we could stop adding new fields to it. > 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 > > > 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15468390#comment-15468390
]
Wangda Tan commented on YARN-5552:
--
Thanks [~Tao Jie] working on this.
I think builder-like API has better readability than newInstance. I also used
builder API while drafting YARN-5478.
But considering overheads to maintain two set of APIs, I think it's better to
deprecate the old API. Is that the plan in your mind? [~asuresh], [~kasha].
And one comment to the patch after a overall review:
- Instead of storing variables locally in the builder, like:
{code}
115 private Priority priority = Priority.newInstance(0);
116 private String resourceName = ResourceRequest.ANY;
117 private Resource capability = null;
{code}
Is it better to create a fresh record with the Builder constructor, and invoke
set... when methods like (priority(...)) is invoked.
> 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
>
>
> 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-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15457799#comment-15457799 ] Tao Jie commented on YARN-5552: --- Failed testcase passed in my local environment. > 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 > > > 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15457470#comment-15457470
]
Hadoop QA commented on YARN-5552:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 19s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m 0s
{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 9s
{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
55s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m 24s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
38s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m 2s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 1m
2s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
29s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 21s
{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 10s
{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
39s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m 23s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 2m 23s
{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m 37s
{color} | {color:red} hadoop-yarn-project/hadoop-yarn: The patch generated 36
new + 115 unchanged - 8 fixed = 151 total (was 123) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 53s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
51s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m
0s {color} | {color:green} The patch has no whitespace issues. {color} |
| {color:red}-1{color} | {color:red} findbugs {color} | {color:red} 0m 38s
{color} | {color:red} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 13s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m 24s
{color} | {color:green} hadoop-yarn-api in the patch passed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 2m 19s
{color} | {color:green} hadoop-yarn-common in the patch passed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 38m 1s {color}
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 16m 5s
{color} | {color:green} hadoop-yarn-client in the patch passed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
18s {color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 89m 10s {color}
| {color:black} {color} |
\\
\\
|| Reason || Tests ||
| FindBugs | module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client |
| |
org.apache.hadoop.yarn.client.api.AMRMClient$ContainerRequest$ContainerRequestBuilder.nodes(String[])
may expose internal representation by storing an externally mutable object
into AMRMClient$ContainerRequest$ContainerRequestBuilder.nodes At
AMRMClient.java:by storing an externally mutable object into
AMRMClient$ContainerRequest$ContainerRequestBuilder.nodes At
AMRMClient.java:[line 398] |
| |
org.apache.hadoop.yarn.client.api.AMRMClient$ContainerRequest$ContainerRequestBuilder
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15457307#comment-15457307 ] Tao Jie commented on YARN-5552: --- Attached patch fixed checkstyle and failed test. [~asuresh], [~kasha], would you mind giving it a review? Thank you! > 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 > > > 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15456046#comment-15456046
]
Hadoop QA commented on YARN-5552:
-
| (x) *{color:red}-1 overall{color}* |
\\
\\
|| Vote || Subsystem || Runtime || Comment ||
| {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 17s
{color} | {color:blue} Docker mode activated. {color} |
| {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s
{color} | {color:green} The patch does not contain any @author tags. {color} |
| {color:red}-1{color} | {color:red} test4tests {color} | {color:red} 0m 0s
{color} | {color:red} The patch doesn't appear to include any new or modified
tests. Please justify why no new tests are needed for this patch. Also please
list what manual steps were performed to verify this patch. {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 9s
{color} | {color:blue} Maven dependency ordering for branch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 6m
53s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m 23s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m
41s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 2m 0s
{color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
59s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 3m
12s {color} | {color:green} trunk passed {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 10s
{color} | {color:green} trunk passed {color} |
| {color:blue}0{color} | {color:blue} mvndep {color} | {color:blue} 0m 9s
{color} | {color:blue} Maven dependency ordering for patch {color} |
| {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 1m
39s {color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} compile {color} | {color:green} 2m 19s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} javac {color} | {color:green} 2m 19s
{color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} checkstyle {color} | {color:red} 0m 40s
{color} | {color:red} hadoop-yarn-project/hadoop-yarn: The patch generated 110
new + 210 unchanged - 8 fixed = 320 total (was 218) {color} |
| {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 1m 52s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} mvneclipse {color} | {color:green} 0m
52s {color} | {color:green} the patch passed {color} |
| {color:red}-1{color} | {color:red} whitespace {color} | {color:red} 0m 0s
{color} | {color:red} The patch has 6 line(s) that end in whitespace. Use git
apply --whitespace=fix. {color} |
| {color:red}-1{color} | {color:red} findbugs {color} | {color:red} 0m 40s
{color} | {color:red} hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client
generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) {color} |
| {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 1m 3s
{color} | {color:green} the patch passed {color} |
| {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m 24s
{color} | {color:green} hadoop-yarn-api in the patch passed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 13m 25s {color}
| {color:red} hadoop-yarn-server-nodemanager in the patch failed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 38m 43s {color}
| {color:red} hadoop-yarn-server-resourcemanager in the patch failed. {color} |
| {color:red}-1{color} | {color:red} unit {color} | {color:red} 10m 59s {color}
| {color:red} hadoop-yarn-client in the patch failed. {color} |
| {color:green}+1{color} | {color:green} asflicense {color} | {color:green} 0m
18s {color} | {color:green} The patch does not generate ASF License warnings.
{color} |
| {color:black}{color} | {color:black} {color} | {color:black} 94m 59s {color}
| {color:black} {color} |
\\
\\
|| Reason || Tests ||
| FindBugs | module:hadoop-yarn-project/hadoop-yarn/hadoop-yarn-client |
| |
org.apache.hadoop.yarn.client.api.AMRMClient$ContainerRequest$ContainerRequestBuilder.nodes(String[])
may expose internal representation by storing an externally mutable object
into AMRMClient$ContainerRequest$ContainerRequestBuilder.nodes At
AMRMClient.java:by storing an externally mutable object into
AMRMClient$ContainerRequest$ContainerRequestBuilder.nodes At
AMRMClient.java:[line 394] |
| |
org.apache.hadoop.yarn.client.api.AMRMClient$Cont
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15454994#comment-15454994 ] Tao Jie commented on YARN-5552: --- Rebased the patch. And find out api records with quite a few properties and multiple newInstance methods to add builder： Container, ApplicationReport, ContainerReport, GetApplicationsRequest > 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 > > > 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[
https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15451609#comment-15451609
]
Tao Jie commented on YARN-5552:
---
Initialized the patch, added builder for mentioned 4 records.
constructors and newInstance methods are still retained to keep compatibility.
Also use {{builder}} instead of {{newInstance}} somewhere, but not in test yet.
> 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
>
>
> 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-unsubscr...@hadoop.apache.org
For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15447810#comment-15447810 ] Arun Suresh commented on YARN-5552: --- Sure [~Tao Jie].. Assigning this to you. Thanks !! > 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 > > 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
[jira] [Commented] (YARN-5552) Add Builder methods for common yarn API records
[ https://issues.apache.org/jira/browse/YARN-5552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15447789#comment-15447789 ] Tao Jie commented on YARN-5552: --- Hi [~asuresh], I would like to take this Jira over, if you have not started it yet. > 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 > > 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-unsubscr...@hadoop.apache.org For additional commands, e-mail: yarn-issues-h...@hadoop.apache.org
