[
https://issues.apache.org/jira/browse/YARN-10002?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17055072#comment-17055072
]
Adam Antal commented on YARN-10002:
-----------------------------------
Thanks for the patch [~bteke]! Looks good overall, I checked the moves to the
base classes and syntactically it looks good.
Minor nits:
- Could you please add a small description on the javadoc of
{{PersistentConfigurationStoreBaseTest}}. It suffice to add a reference to the
two extending classes.
- In {{TestFSSchedulerConfigurationStore#prepareParameterizedLogMutation}}
variadic function I'd explicitly check whether we have even number of inputs
and throw exception otherwise.
- In {{testVersionHelper}} I can still see some functionality that we can pull
into {{PersistentConfigurationStoreBaseTest}}. I suggest to create an abstract
method, {{#getVersion()}} that should be implemented in the extending classes.
In this way, we can put {{@Test}} annotation to the test (and we don't need the
input parameter).
On a side note, I think {{Supplier}} was a really good construct for
implementing this test, but I have the impression that {{#getLogs()}} should be
the part of the {{YarnConfigurationStore}} abstract class.
> Code cleanup and improvements ConfigurationStoreBaseTest
> --------------------------------------------------------
>
> Key: YARN-10002
> URL: https://issues.apache.org/jira/browse/YARN-10002
> Project: Hadoop YARN
> Issue Type: Improvement
> Reporter: Szilard Nemeth
> Assignee: Benjamin Teke
> Priority: Minor
> Attachments: YARN-10002.001.patch, YARN-10002.002.patch,
> YARN-10002.003.patch, YARN-10002.004.patch, YARN-10002.005.patch
>
>
> * Some protected fields could be package-private
> * Could add a helper method that prepares a simple LogMutation with 1, 2 or 3
> updates (Key + value) as this pattern is used extensively in subclasses
--
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
