[ https://issues.apache.org/jira/browse/SUBMARINE-47?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16803437#comment-16803437 ]
Hadoop QA commented on SUBMARINE-47: ------------------------------------ | (x) *{color:red}-1 overall{color}* | \\ \\ || Vote || Subsystem || Runtime || Comment || | {color:blue}0{color} | {color:blue} reexec {color} | {color:blue} 0m 45s{color} | {color:blue} Docker mode activated. {color} | || || || || {color:brown} Prechecks {color} || | {color:green}+1{color} | {color:green} dupname {color} | {color:green} 0m 1s{color} | {color:green} No case conflicting files found. {color} | | {color:blue}0{color} | {color:blue} yamllint {color} | {color:blue} 0m 0s{color} | {color:blue} yamllint was not available. {color} | | {color:green}+1{color} | {color:green} @author {color} | {color:green} 0m 0s{color} | {color:green} The patch does not contain any @author tags. {color} | | {color:green}+1{color} | {color:green} test4tests {color} | {color:green} 0m 0s{color} | {color:green} The patch appears to include 11 new or modified test files. {color} | || || || || {color:brown} trunk Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 25m 18s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 22s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} checkstyle {color} | {color:green} 0m 19s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 28s{color} | {color:green} trunk passed {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 12m 49s{color} | {color:green} branch has no errors when building and testing our client artifacts. {color} | | {color:red}-1{color} | {color:red} findbugs {color} | {color:red} 0m 36s{color} | {color:red} branch/hadoop-submarine/hadoop-submarine-core cannot run setBugDatabaseInfo from findbugs {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 22s{color} | {color:green} trunk passed {color} | || || || || {color:brown} Patch Compile Tests {color} || | {color:green}+1{color} | {color:green} mvninstall {color} | {color:green} 0m 21s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} compile {color} | {color:green} 0m 20s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javac {color} | {color:green} 0m 20s{color} | {color:green} the patch passed {color} | | {color:orange}-0{color} | {color:orange} checkstyle {color} | {color:orange} 0m 13s{color} | {color:orange} hadoop-submarine/hadoop-submarine-core: The patch generated 82 new + 38 unchanged - 8 fixed = 120 total (was 46) {color} | | {color:green}+1{color} | {color:green} mvnsite {color} | {color:green} 0m 23s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} whitespace {color} | {color:green} 0m 0s{color} | {color:green} The patch has no whitespace issues. {color} | | {color:green}+1{color} | {color:green} xml {color} | {color:green} 0m 2s{color} | {color:green} The patch has no ill-formed XML file. {color} | | {color:green}+1{color} | {color:green} shadedclient {color} | {color:green} 13m 29s{color} | {color:green} patch has no errors when building and testing our client artifacts. {color} | | {color:green}+1{color} | {color:green} findbugs {color} | {color:green} 0m 36s{color} | {color:green} the patch passed {color} | | {color:green}+1{color} | {color:green} javadoc {color} | {color:green} 0m 17s{color} | {color:green} the patch passed {color} | || || || || {color:brown} Other Tests {color} || | {color:green}+1{color} | {color:green} unit {color} | {color:green} 0m 31s{color} | {color:green} hadoop-submarine-core in the patch passed. {color} | | {color:red}-1{color} | {color:red} asflicense {color} | {color:red} 0m 32s{color} | {color:red} The patch generated 7 ASF License warnings. {color} | | {color:black}{color} | {color:black} {color} | {color:black} 58m 10s{color} | {color:black} {color} | \\ \\ || Subsystem || Report/Notes || | Docker | Client=17.05.0-ce Server=17.05.0-ce base: https://builds.apache.org/job/PreCommit-SUBMARINE-Build/30/artifact/out/Dockerfile | | JIRA Issue | SUBMARINE-47 | | JIRA Patch URL | https://issues.apache.org/jira/secure/attachment/12963958/SUBMARINE-47.003.patch | | Optional Tests | dupname asflicense compile javac javadoc mvninstall mvnsite unit shadedclient xml findbugs checkstyle yamllint | | uname | Linux 8b53d542fc63 4.4.0-138-generic #164~14.04.1-Ubuntu SMP Fri Oct 5 08:56:16 UTC 2018 x86_64 x86_64 x86_64 GNU/Linux | | Build tool | maven | | Personality | personality/hadoop.sh | | git revision | trunk / 9cd6619 | | maven | version: Apache Maven 3.3.9 | | Default Java | 1.8.0_191 | | findbugs | https://builds.apache.org/job/PreCommit-SUBMARINE-Build/30/artifact/out/branch-findbugs-hadoop-submarine_hadoop-submarine-core.txt | | checkstyle | https://builds.apache.org/job/PreCommit-SUBMARINE-Build/30/artifact/out/diff-checkstyle-hadoop-submarine_hadoop-submarine-core.txt | | Test Results | https://builds.apache.org/job/PreCommit-SUBMARINE-Build/30/testReport/ | | asflicense | https://builds.apache.org/job/PreCommit-SUBMARINE-Build/30/artifact/out/patch-asflicense-problems.txt | | Max. process+thread count | 293 (vs. ulimit of 10000) | | modules | C: hadoop-submarine/hadoop-submarine-core U: hadoop-submarine/hadoop-submarine-core | | Console output | https://builds.apache.org/job/PreCommit-SUBMARINE-Build/30/console | | Powered by | Apache Yetus 0.10.0-SNAPSHOT http://yetus.apache.org | This message was automatically generated. > Provide an implementation to parse configuration values from a YAML file > ------------------------------------------------------------------------ > > Key: SUBMARINE-47 > URL: https://issues.apache.org/jira/browse/SUBMARINE-47 > Project: Hadoop Submarine > Issue Type: New Feature > Reporter: Szilard Nemeth > Assignee: Szilard Nemeth > Priority: Major > Attachments: SUBMARINE-47.001.patch, SUBMARINE-47.002.patch, > SUBMARINE-47.002.patch, SUBMARINE-47.003.patch, SUBMARINE-47.003.patch, > valid-config.yaml > > > In the Submarine design doc > ([https://docs.google.com/document/d/199J4pB3blqgV9SCNvBbTqkEoQdjoyGMjESV4MktCo0k/edit#heading=h.klmv5hrbgx9l]), > under section "Job commands" (submarine job run), the document talks about > the need to implement a YAML parser as an alternative configuration source of > Submarine. > This jira provides an implementation of this YAML configuration parser. > *Some details on the implementation:* > *Data model and YAML library:* > SnakeYAML was the library of choice, as we already include it in other > Hadoop maven modules. > SnakeYAML requires data classes to parse the YAML structure into Java > classes, this is very similar to parsing JSON data. > The data classes are under this package: > org.apache.hadoop.yarn.submarine.client.cli.param.yaml > The data model is the following: > 1. On the root level, we have the class YamlConfigFile. This class has > fields for configs, roles, scheduling, etc. > 2. Class Configs is a class that stores various config values. > 3. We have the class Roles, that stores the role configurations, currently > we only have Worker and Ps (ParameterServer). > 4. There's a base class for Roles, called Role. For some reasons, SnakeYAML > did not like this class as an abstract class, but it's not that important to > have it like that, so I stopped caring. > There are several other data classes, please note that all of these classes > are trying to "mirror" the example YAML file from the design doc, so you can > check that for more details. > *Testing:* > Added a new class called TestRunJobCliParsingYaml that exclusively verifies > the correctness of YAML parsing. > I tried to define the names of the testcases as descriptive and easy to > understand as possible. > Covering some edge cases was also a quite important factor, like checking > how parsing handles non-existing files, empty files, etc. > *ParameterHolder:* > In order to minimize changes in RunJobParameters, I introduced a > ParameterHolder class that is passed > RunJobParameters#updateParametersByParsedCommandline instead of the parsed > CLI object we had before. This way, I could use the ParameterHolder to store > the parsed CLI values along with parsed YAML values, so it's a single source > of configuration. This class can act as a "decision point" about future needs > on value precedences: For example, if we want to have precedence for some > configs coming from the CLI over the same configs coming from the YAML file > (or vica-versa), it is easily doable with the current implementation. > Currently, all values coming from the YAML config are in precedence. > *What CLI options are covered by YAML:* > Every CLI option have it's YAML counterpart, so the coverage is 100%. > See this method for all the available options: RunJobCli#generateOptions. > However, the design doc contains 2 fields (called reservation and placement) > under the 'scheduling' section that are not added to the YAML data classes. > The reason for this is that I haven't found any implementation for these > properties with the original CLI parser, either. > *Questions:* > 1. Do we want to allow mixing of CLI configs and YAML configs? We could > either fail-fast if a config is defined twice. With my current > implementation, the values coming from YAML always have precedence over CLI > values, so defining a config twice is not an error case yet. > 2. What types of roles should we allow? Is there any additional apart from > Worker and Ps? > -- This message was sent by Atlassian JIRA (v7.6.3#76005)