[
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: [email protected]
For additional commands, e-mail: [email protected]
