> On Oct. 16, 2015, 9:24 a.m., Pallavi Rao wrote: > > unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java, line 203 > > <https://reviews.apache.org/r/39316/diff/1/?file=1098225#file1098225line203> > > > > 1 instance sufficient?
I want to make sure Coordinator is in running state while deleting. Anyway we are deleting the process in test case , i think 10 would be ok. > On Oct. 16, 2015, 9:24 a.m., Pallavi Rao wrote: > > unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java, line 153 > > <https://reviews.apache.org/r/39316/diff/1/?file=1098225#file1098225line153> > > > > Since you are testing scheduling, can you just schedule 1 instance, so > > the test is faster? Just making sure Coordinator is in running state . And it's not waiting until 10 instances complete, it will wait only for one instance to complete. > On Oct. 16, 2015, 9:24 a.m., Pallavi Rao wrote: > > unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java, line 336 > > <https://reviews.apache.org/r/39316/diff/1/?file=1098224#file1098224line336> > > > > Isn't enum better? waitForStatus will finally call getStatusOfInstances method which accepts entity type as string . To maintain consistency it has changed to String. > On Oct. 16, 2015, 9:24 a.m., Pallavi Rao wrote: > > unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java, line 231 > > <https://reviews.apache.org/r/39316/diff/1/?file=1098222#file1098222line231> > > > > Why has this been changed to String? Isn't it better to use an enum? This method will call getStatusOfInstances which is in FalconClient. to maintain consistency across all methods it was changed. > On Oct. 16, 2015, 9:24 a.m., Pallavi Rao wrote: > > common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java, > > line 63 > > <https://reviews.apache.org/r/39316/diff/1/?file=1098217#file1098217line63> > > > > Not used by the ConfigurationStore. Better if it's moved to > > FalconUnitTestBase Entity Load Order is available in Configuration Store. I thought it will be good to maintain even deletion oredr at same place even though it won't be used as of now. > On Oct. 16, 2015, 9:24 a.m., Pallavi Rao wrote: > > prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java, > > line 283 > > <https://reviews.apache.org/r/39316/diff/1/?file=1098221#file1098221line283> > > > > BufferedRequest is no longer needed? Yes, BufferedRequest is not used anywhere in delete method. > On Oct. 16, 2015, 9:24 a.m., Pallavi Rao wrote: > > prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java, > > line 289 > > <https://reviews.apache.org/r/39316/diff/1/?file=1098221#file1098221line289> > > > > Why is it changed to plain request? Anyway bufferedRequest won't be used in delete method . Will fix it . - pavan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39316/#review102894 ----------------------------------------------------------- On Oct. 14, 2015, 1:06 p.m., pavan kumar kolamuri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39316/ > ----------------------------------------------------------- > > (Updated Oct. 14, 2015, 1:06 p.m.) > > > Review request for Falcon. > > > Bugs: https://issues.apache.org/jira/browse/FALCON-1520 > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/FALCON-1520 > > > Repository: falcon-git > > > Description > ------- > > Added Entity API's in Falcon Unit to support for Integration tests > > > Diffs > ----- > > client/src/main/java/org/apache/falcon/client/AbstractFalconClient.java > b889931 > client/src/main/java/org/apache/falcon/client/FalconCLIException.java > ec74c27 > common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java > e27187b > oozie/src/main/java/org/apache/oozie/client/LocalProxyOozieClient.java > 756828f > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java > 3323dd1 > > prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java > 0db55df > > prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java > 9d13d74 > unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java 783af19 > > unit/src/main/java/org/apache/falcon/unit/LocalSchedulableEntityManager.java > 8b1c435 > unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java d12efbc > unit/src/test/java/org/apache/falcon/unit/TestFalconUnit.java d504bd2 > unit/src/test/resources/process1.xml 37dbb9c > > Diff: https://reviews.apache.org/r/39316/diff/ > > > Testing > ------- > > Added Unit Tests for these API's > > > Thanks, > > pavan kumar kolamuri > >
