[jira] [Commented] (YARN-9052) Replace all MockRM submit method definitions with a builder

["Adam Antal (JIRA)" <ji...@apache.org>]

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

Adam Antal commented on YARN-9052:
----------------------------------

Thanks for the patch [~snemeth], massive one! I wanted to have some more meaningful comments, but I can't effectively analyse your patch without an IDE, and it looks like your patch does not apply to current trunk. This is not suprising as you have touched a lot of files.

- Please rebase (I am sorry)
- The patch still has 119 new checkstyle issues.
- In {{testValidateRequestCapacityAgainstMinMaxAllocation}}, {{testRequestCapacityMinMaxAllocationForResourceTypes}} no need to create a separate {{MockRMAppSubmissionData}} object (you can just create right like you did other time the {{MockRM}} creation by the {{MockRMAppSubmitter}})
- In {{security/TestDelegationTokenRenewer.java}} you do not need the {{Resource}} object - the call with the memory automatically does what you want. Also: I could not read your whole patch, but do you actually need the {{createWithDefaults}} with the {{Resource}} object? I see all the time using only the memory.
Speaking about *Builders*: I think there should be only one {{createWithDefaults}} function, which accepts only one parameter: a {{MockRM}}. The other two type of the function (called with memory or resource) can be added like the other parameters: creating a {{withMemory}} and a {{withResource}} function that provides these parameters to the builder. If it is required by logic of the class that one of those must be given, then this condition should be checked in the build() function. 

> 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
>         Attachments: YARN-9052.001.patch, YARN-9052.002.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
(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