----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65636/#review214235 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java Line 138 (original), 138 (patched) <https://reviews.apache.org/r/65636/#comment300425> We could use the diamond operator here. core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java Lines 146 (patched) <https://reviews.apache.org/r/65636/#comment300430> foreach would be nicer core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java Lines 162-164 (patched) <https://reviews.apache.org/r/65636/#comment300426> for (Path configDefaultFile : configDefaults) would be nicer. core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java Lines 168 (patched) <https://reviews.apache.org/r/65636/#comment300427> Why is it System.out instead of LOG? core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java Line 156 (original), 177 (patched) <https://reviews.apache.org/r/65636/#comment300428> I think it would be useful to add configDefaultFile to the error message. core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java Line 159 (original), 180 (patched) <https://reviews.apache.org/r/65636/#comment300431> As far as I understand in the following scenario: i=0 and fs.exists(configDefaultFile) == true: we create a new defaultConfigs i=1 and fs.exists(configDefaultFile) == false: we skip the if in line 167 and we don't create a new defaultConfigs. In this case the if in line 180 will refer to the old defaultConfigs and call copy again using the old data. Could you please check this scenario and fix it. core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java Lines 506-516 (patched) <https://reviews.apache.org/r/65636/#comment300429> Please add assert messages to the assertEquals. - Andras Salamon On March 29, 2019, 1 p.m., Jason Phelps wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65636/ > ----------------------------------------------------------- > > (Updated March 29, 2019, 1 p.m.) > > > Review request for oozie. > > > Repository: oozie-git > > > Description > ------- > > Added another client property: `oozie.default.configuration.path` which > allows for a comma seperated list of HDFS XML property files to be applied to > the job. This is useful if multiple workflows have shared properties (i.e. > database names, table names for Sqoop or Hive, spark arguments, etc). This is > similar to the `oozie.libpath` property which allows user's to define common > paths workflows to load jars. > > > Diffs > ----- > > client/src/main/java/org/apache/oozie/client/OozieClient.java 2862d33f > core/src/main/java/org/apache/oozie/command/wf/SubmitXCommand.java 70b9adc1 > core/src/test/java/org/apache/oozie/command/wf/TestSubmitXCommand.java > 2bc0baa4 > > > Diff: https://reviews.apache.org/r/65636/diff/3/ > > > Testing > ------- > > > Thanks, > > Jason Phelps > >
