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

Benjamin Teke commented on YARN-10003:
--------------------------------------

Hi [~adam.antal],

Thanks for the review, makes sense, updated the patch based on the comments. 

About this: 
 - I don't see why we catch the exception in 
{{TestInMemoryConfigurationStore#checkVersion}} - if an exception is thrown 
test is failed anyways, and the {{Assert#fail}} method does not give further 
information.

This fail serves only the purpose of easy understanding. Based on a best 
practice 
([https://pmd.github.io/latest/pmd_rules_java_bestpractices.html#junittestsshouldincludeassert])
 it shows that the unittest is testing that the method does not throw any 
exception. But if it's unnecessary I could remove it, of course.

> YarnConfigurationStore#checkVersion throws exception that belongs to 
> RMStateStore
> ---------------------------------------------------------------------------------
>
>                 Key: YARN-10003
>                 URL: https://issues.apache.org/jira/browse/YARN-10003
>             Project: Hadoop YARN
>          Issue Type: Improvement
>            Reporter: Szilard Nemeth
>            Assignee: Benjamin Teke
>            Priority: Major
>         Attachments: YARN-10003.001.patch, YARN-10003.002.patch, 
> YARN-10003.003.patch, YARN-10003.004.patch
>
>
> RMStateVersionIncompatibleException is thrown from method "checkVersion".
> Moreover, there's a TODO here saying this method is copied from RMStateStore. 
> We should revise this method a bit.



--
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