hadoop-yarn-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Antal Bálint Steinbach (JIRA) <j...@apache.org>
Subject [jira] [Comment Edited] (YARN-8553) Reduce complexity of AHSWebService getApps method
Date Tue, 18 Sep 2018 15:00:00 GMT

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

Antal Bálint Steinbach edited comment on YARN-8553 at 9/18/18 2:59 PM:
-----------------------------------------------------------------------

Hi [~snemeth] ,

Thanks for creating the patch. Your changes cleaning the code for sure.

I found some minor things to consider:

1) I would set the boolean fields default value in _ApplicationsRequestBuilderCommons_ to
false. It is more straightforward in that way.
{code:java}
private boolean startedTimeSpecified;
private boolean finishedTimeSpecified;
private boolean appStatesSpecified;
private boolean appTypesSpecified;{code}
2) in _ApplicationsRequestBuilderCommons_ 
{code:java}
Set<String> appStates = ApplicationsRequestValueParser
        .parseApplicationStates(this.appStates);
{code}
appStates is already a field in the class.

3) _TestApplicationsRequestBuilderCommons_ it would be easier to maintain and read if you
don't check all the "builder setters" one by one. In my opinion, it is enough to keep the
one which tests all of them together, plus the ones which are testing edge cases or invalid
values.


was (Author: bsteinbach):
Hi [~snemeth] ,

Thanks for creating the patch. Your changes cleaning the code for sure.

I found some minor things to consider:

1) I would set the boolean fields default value in _ApplicationsRequestBuilderCommons_ to
false. It is more straightforward in that way.
{code:java}
private boolean startedTimeSpecified;
private boolean finishedTimeSpecified;
private boolean appStatesSpecified;
private boolean appTypesSpecified;{code}
2) in _ApplicationsRequestBuilderCommons_ 
{code:java}
// Set<String> appStates = ApplicationsRequestValueParser
        .parseApplicationStates(this.appStates);
{code}
appStates is already a field in the class.

3) _TestApplicationsRequestBuilderCommons_ it would be easier to maintain and read if you
don't check all the "builder setters" one by one. In my opinion, it is enough to keep the
one which tests all of them together, plus the ones which are testing edge cases or invalid
values.

> Reduce complexity of AHSWebService getApps method
> -------------------------------------------------
>
>                 Key: YARN-8553
>                 URL: https://issues.apache.org/jira/browse/YARN-8553
>             Project: Hadoop YARN
>          Issue Type: Bug
>            Reporter: Rohith Sharma K S
>            Assignee: Szilard Nemeth
>            Priority: Major
>         Attachments: YARN-8553.001.patch, YARN-8553.001.patch, YARN-8553.002.patch
>
>
> YARN-8501 refactor the RMWebService#getApp. Similar refactoring required in AHSWebservice.




--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

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


Mime
View raw message