[
https://issues.apache.org/jira/browse/YARN-10003?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17052141#comment-17052141
]
Benjamin Teke edited comment on YARN-10003 at 3/5/20, 1:39 PM:
---------------------------------------------------------------
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 in fact testing that the method does not throw
any exception. But if it's unnecessary I could remove it, of course.
was (Author: bteke):
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
