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

Gergely Pollak commented on YARN-10278:
---------------------------------------

Thank you [~snemeth] for the patch, it is already much better. There are still 
some improvements to do, but I think we should handle those cases in separate 
JIRAS. My only suggestion, which might be worth to include in this patch, is to 
remove the magic numbers which are used as indexes during the parsing. If we'd 
used constants instead of just numbers, we wouldn't need to look for the 
nearest comment to figure out the order of the arguments. But even without this 
change I think it's fine LGTM + 1 (Non-binding)

> CapacityScheduler test framework 
> ProportionalCapacityPreemptionPolicyMockFramework need some review
> ---------------------------------------------------------------------------------------------------
>
>                 Key: YARN-10278
>                 URL: https://issues.apache.org/jira/browse/YARN-10278
>             Project: Hadoop YARN
>          Issue Type: Task
>            Reporter: Gergely Pollak
>            Assignee: Szilard Nemeth
>            Priority: Major
>         Attachments: YARN-10278.001.patch, YARN-10278.002.patch
>
>
> This test framework class mocks a bit too heavily, and simulates CS internal 
> behaviour with the mock methods over a point it is reasonably maintainable, 
> any internal change in CS is a major headscratch.
> A lot of tests depend on this class, so we should approach it carefully, but 
> I think it's wroth to examine this class if it can be made a bit more 
> resilient to changes, and easier to maintain. Or at least document it better.



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

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to