> On Sept. 11, 2015, 3:28 p.m., Balu Vellanki wrote: > > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 904 > > <https://reviews.apache.org/r/38294/diff/1/?file=1068154#file1068154line904> > > > > Do you not allow ":" character in the value? I see value in allowing > > ":" in the value for properties. Splitting kvPair into two Strings might be > > more helpful. > > > > String[] keyValue = kvPair.trim().split(":", 2);
Makes sense. Will allow : in values. > On Sept. 11, 2015, 3:28 p.m., Balu Vellanki wrote: > > common/src/main/java/org/apache/falcon/entity/EntityUtil.java, line 905 > > <https://reviews.apache.org/r/38294/diff/1/?file=1068154#file1068154line905> > > > > This can allow the keyValue[0] to be empty. I think we should not have > > empty property keys. Good catch. Added an additional check. > On Sept. 11, 2015, 3:28 p.m., Balu Vellanki wrote: > > common/src/test/java/org/apache/falcon/entity/EntityUtilTest.java, line 337 > > <https://reviews.apache.org/r/38294/diff/1/?file=1068156#file1068156line337> > > > > Please add " :value1" to list of invalid props. Done. > On Sept. 11, 2015, 3:28 p.m., Balu Vellanki wrote: > > client/src/main/java/org/apache/falcon/cli/FalconCLI.java, line 94 > > <https://reviews.apache.org/r/38294/diff/1/?file=1068151#file1068151line94> > > > > Minor nit - Can we use "properties" instead? > > > > Can we also add some tests to EntityManagerJerseyIT? Changed to "properties". Just enhanced the existing tests in FalconCLIIT and EntityManagerJerseyIT. Didn't add any new test as the properties aren't really used yet. - Pallavi ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38294/#review98613 ----------------------------------------------------------- On Sept. 14, 2015, 6:21 a.m., Pallavi Rao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38294/ > ----------------------------------------------------------- > > (Updated Sept. 14, 2015, 6:21 a.m.) > > > Review request for Falcon. > > > Bugs: FALCON-1434 > https://issues.apache.org/jira/browse/FALCON-1434 > > > Repository: falcon-git > > > Description > ------- > > The schedule API will be enhanced to accept a key-value properties. This is a > foundation to enable users to specify the scheduler on which they want to > schedule the entity. This in turn enables migration to native scheduler from > Oozie. > Example: > bin/falcon entity -schedule -props falcon.scheduler=native -name <entity name> > > > Diffs > ----- > > client/src/main/java/org/apache/falcon/cli/FalconCLI.java d4da302 > client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java > 282b41b > client/src/main/java/org/apache/falcon/client/FalconClient.java 44436d2 > common/src/main/java/org/apache/falcon/entity/EntityUtil.java 25d9008 > > common/src/main/java/org/apache/falcon/workflow/engine/AbstractWorkflowEngine.java > ea86c2a > common/src/test/java/org/apache/falcon/entity/EntityUtilTest.java cfdc84d > > oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java > 5f79ca1 > > prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java > f9405dc > > prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java > ceabb06 > unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java eb65cb3 > unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java 997b301 > unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java 498f50e > > webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java > 1f8cc1b > webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java 0062070 > webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java > bcd3bd5 > webapp/src/test/java/org/apache/falcon/resource/TestContext.java 54671fb > > Diff: https://reviews.apache.org/r/38294/diff/ > > > Testing > ------- > > UT added > Manually tested to ensure CLI accepts properties and it is propagated. > > > Thanks, > > Pallavi Rao > >
