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

Ahmed Hussein commented on YARN-9052:
-------------------------------------

[~sunilg], [~snemeth], [~shuzirra], and [~adam.antal]

Thanks for the effort cleaning up {{MockRM}}.

I am a little bit concerned with Jiras that solely opened for the purpose of 
refactoring and code readability; especially when they have no impact on bug 
fixes fix/performance.
While this patch could be needed to make MockRM easier to use, 
"code-readability" is subjective in general. It will trigger infinite set of 
unnecessary code commits. 
# Hadoop code is too big to open Jiras only for the purpose of refactoring and 
that will not scale for such a large open source community.
# The patch here touches so many files and changes tens of Junit tests.
# Any developer who has a pending patch that includes {{submitApp()}} in new 
tests cases (i.e., MAPREDUCE-7169) will have to go through his changes one more 
time to upload a new patch.
# This patch creates many conflicts with other branches and complicates merges 
going forward. This is a significant amount of man hours to handle the new 
patch.

cc: [~nroberts], [~Jim_Brennan]

> Replace all MockRM submit method definitions with a builder
> -----------------------------------------------------------
>
>                 Key: YARN-9052
>                 URL: https://issues.apache.org/jira/browse/YARN-9052
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Szilard Nemeth
>            Assignee: Szilard Nemeth
>            Priority: Minor
>             Fix For: 3.3.0
>
>         Attachments: 
> YARN-9052-004withlogs-patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt,
>  YARN-9052-testlogs003-justfailed.txt, 
> YARN-9052-testlogs003-patch-unit-hadoop-yarn-project_hadoop-yarn_hadoop-yarn-server_hadoop-yarn-server-resourcemanager.txt,
>  YARN-9052-testlogs004-justfailed.txt, YARN-9052.001.patch, 
> YARN-9052.002.patch, YARN-9052.003.patch, YARN-9052.004.patch, 
> YARN-9052.004.withlogs.patch, YARN-9052.005.patch, YARN-9052.006.patch, 
> YARN-9052.007.patch, YARN-9052.008.patch, YARN-9052.009.patch, 
> YARN-9052.009.patch, YARN-9052.testlogs.002.patch, 
> YARN-9052.testlogs.002.patch, YARN-9052.testlogs.003.patch, 
> YARN-9052.testlogs.patch
>
>
> MockRM has 31 definitions of submitApp, most of them having more than 
> acceptable number of parameters, ranging from 2 to even 22 parameters, which 
> makes the code completely unreadable.
> On top of unreadability, it's very hard to follow what RmApp will be produced 
> for tests as they often pass a lot of empty / null values as parameters.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

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

Reply via email to