> On July 26, 2019, 4:05 p.m., Andras Salamon wrote: > > core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java > > Lines 638 (patched) > > <https://reviews.apache.org/r/71168/diff/1/?file=2157981#file2157981line638> > > > > 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.
I think this kind of work is out of scope of this fix. Also number of file systems which shall have custom properties are quite few so in my opinion defining the options in oozie-default.xml with description will help the user to set the configuration more easily. > On July 26, 2019, 4:05 p.m., Andras Salamon wrote: > > core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java > > Lines 646 (patched) > > <https://reviews.apache.org/r/71168/diff/1/?file=2157981#file2157981line646> > > > > 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? This is documented now, we will not allow to use comma in property values nor in property names. > On July 26, 2019, 4:05 p.m., Andras Salamon wrote: > > core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java > > Lines 648 (patched) > > <https://reviews.apache.org/r/71168/diff/1/?file=2157981#file2157981line648> > > > > 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. You are correct. In case there is a property which contains equal sign, then from the second one will be part of the *value*. Please see the update patch. > On July 26, 2019, 4:05 p.m., Andras Salamon wrote: > > core/src/test/java/org/apache/oozie/service/TestHadoopAccessorService.java > > Lines 358 (patched) > > <https://reviews.apache.org/r/71168/diff/1/?file=2157982#file2157982line358> > > > > Please add a few properties to this empty configuration. It would > > improve this test. I do not really think that here we shall add more properties. The purpose of the test is to check the configuration object is untouched. - Denes ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71168/#review216891 ----------------------------------------------------------- On July 26, 2019, 1:43 p.m., Denes Bodo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71168/ > ----------------------------------------------------------- > > (Updated July 26, 2019, 1:43 p.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 > >
