[
https://issues.apache.org/jira/browse/SUBMARINE-49?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16815445#comment-16815445
]
Szilard Nemeth commented on SUBMARINE-49:
-----------------------------------------
Hi [~adam.antal]!
Thanks for the review comments!
1. Filed SUBMARINE-57 for this.
2. Added javadoc to all new classes.
3. The introduced xxParameter classes are mostly different in terms of parsing
logic. Also, the parse method's syntax is different sometimes. Since these are
simple value classes with some parsing logic on the top, I wouldn't bother to
find common behaviour in them, as there's not much.
4. Good point, added some testcases for the xxParameter classes. As
SecurityParameters and RoleResourceParser do not have any meaningful logic, I
haven't created a test class for this one.
5. Thanks :) Which methods do you want to add javadoc to? Please put a list
together and I will add javadocs to them.
6. This was because of an inconsistent usage of quote or the absence of quote
for the parameter name, I just fixed that to be consistent.
7. Added some explanation to PS, FYI PS stands for Parameter server, see:
https://www.tensorflow.org/guide/extend/architecture
8. Indentation issues: Yep, unfortunately my formatter can't deal with some of
the multiline indentations, I don't really know why so I had to manually format
these.
Fixed the checkstyle issues as well, please check the latest patch!
Thanks!
> Add more test coverage to RunJobParameters
> ------------------------------------------
>
> Key: SUBMARINE-49
> URL: https://issues.apache.org/jira/browse/SUBMARINE-49
> Project: Hadoop Submarine
> Issue Type: Improvement
> Reporter: Szilard Nemeth
> Assignee: Szilard Nemeth
> Priority: Minor
> Attachments: SUBMARINE-49.001.patch
>
>
> There are some good tests in
> {{org.apache.hadoop.yarn.submarine.client.cli.TestRunJobCliParsing}}, but
> these are not testing all fields set by method
> {{org.apache.hadoop.yarn.submarine.client.cli.param.RunJobParameters#updateParametersByParsedCommandline}}.
>
> Some more extensive testing is needed in this area.
> As an added bonus, the code
> {{org.apache.hadoop.yarn.submarine.client.cli.param.RunJobParameters#updateParametersByParsedCommandline}}
> could be cleaned up a bit.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
