----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71168/#review216891 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java Lines 638 (patched) <https://reviews.apache.org/r/71168/#comment304133> The property name contains the uri scheme, so it will not be possible to list all the possible properties in the oozie-default.xml. It means that we will get lots of "Invalid configuration defined" warning messages (see OOZIE-2338). Could you modify ConfigurationService.verifyConfigurationName() just like in the OOZIE-2338 do ingore the warnings. core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java Lines 646 (patched) <https://reviews.apache.org/r/71168/#comment304131> What if a property value contains ","? This solution does not allow that. I'm not familiar with the fs properties. Can we safely assume that there is no , is the values? core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java Lines 647 (patched) <https://reviews.apache.org/r/71168/#comment304132> Could we find a more java friendly variable name? core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java Lines 648 (patched) <https://reviews.apache.org/r/71168/#comment304130> We should not expect that the entry contains an = sign. Without the = we get ArrayOutOfBoundsException. What if it's a=b=c? Should we just ignore the =c part? At least we should print out a warning. Maybe we can just give an error and ignore the whole parameter. The solution also assumes that there are no = character in the value. core/src/test/java/org/apache/oozie/service/TestHadoopAccessorService.java Lines 355 (patched) <https://reviews.apache.org/r/71168/#comment304135> Please add a few test cases with invalid values. For instance one without the = sign should check if we avoid ArrayOutOfBoundsException. core/src/test/java/org/apache/oozie/service/TestHadoopAccessorService.java Lines 358 (patched) <https://reviews.apache.org/r/71168/#comment304134> Please add a few properties to this empty configuration. It would improve this test. - Andras Salamon On July 26, 2019, 11:43 a.m., Denes Bodo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71168/ > ----------------------------------------------------------- > > (Updated July 26, 2019, 11:43 a.m.) > > > Review request for oozie. > > > Bugs: OOZIE-3529 > https://issues.apache.org/jira/browse/OOZIE-3529 > > > Repository: oozie-git > > > Description > ------- > > Many customer who uses s3 file system as secondary one experiences > "UnsupportedOperationException: Accessing local file system is not allowed" > error when Oozie tries to submit the Yarn application. > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java > 0b53a3611 > core/src/test/java/org/apache/oozie/service/TestHadoopAccessorService.java > 89ce18550 > > > Diff: https://reviews.apache.org/r/71168/diff/1/ > > > Testing > ------- > > > Thanks, > > Denes Bodo > >
