> On Dec. 14, 2015, 10:11 a.m., Ajay Yadava wrote: > > oozie/src/main/java/org/apache/oozie/client/LocalProxyOozieClient.java, > > line 96 > > <https://reviews.apache.org/r/40769/diff/2/?file=1162121#file1162121line96> > > > > I know it's not part of this JIRA, but we should make the naming > > consistent between bundle, coord and oozie client. > > > > getLocalOozieCoordinatorClient etc. I hope it's a very trivial change > > using IDE refactoring. > > pavan kumar kolamuri wrote: > It was added like this because LocalOozieClientCoord is already there in > oozie , just to give an understanding that we are proxying this it was made > like that. > > Ajay Yadava wrote: > I don't mind suffix "Proxy" it should just be used consistently. Anyhow, > I believe this is needed to unblock other patches, so may be you can address > it next time.
ok Thanks. > On Dec. 14, 2015, 10:11 a.m., Ajay Yadava wrote: > > unit/src/test/java/org/apache/falcon/unit/examples/JavaHelloWorldExample.java, > > line 23 > > <https://reviews.apache.org/r/40769/diff/2/?file=1162130#file1162130line23> > > > > Please add more documentation on how to use it and mention some example > > test cases using it. > > pavan kumar kolamuri wrote: > Sorry, i didn't understand. May be comment at wrong place > > Ajay Yadava wrote: > I was just suggesting that the comment is terse and that combined with > name is confusing on purpose of this class. More details in documentation for > purpose and usage examples of this class. e.g. > > This is a dummy class to be used in Java Action of Falcon Unit tests. To > use this... , you can see an example of this at ... I feel all examples should be at same place, based on the use case in tests users should pick it . For some cases workflow need to be succedded for that hello world example is used. For some cases workflow should be in running then sleep example is required. The same example can be used across different places i don't think it will be good to mention all use cases here. > On Dec. 14, 2015, 10:11 a.m., Ajay Yadava wrote: > > webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java, > > line 75 > > <https://reviews.apache.org/r/40769/diff/2/?file=1162134#file1162134line75> > > > > Why not use submitAndSchedule, since these are not the focus of this > > test? > > pavan kumar kolamuri wrote: > We want to control no of instances during scheduling process, thats why > used submit and schedule separateley > > Ajay Yadava wrote: > May be, we should make a submitAndSchedule variant which accepts number > of instances. Thoughts? This will make the code much smaller and the core > logic of test more readable. I still feel controlling no of instances should be at scheduling level, it is not submit level parameter. I agree code should be smaller and core logic of test more readable , we can refactor later as part of https://issues.apache.org/jira/browse/FALCON-1659 > On Dec. 14, 2015, 10:11 a.m., Ajay Yadava wrote: > > webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java, > > lines 93-106 > > <https://reviews.apache.org/r/40769/diff/2/?file=1162134#file1162134line93> > > > > This code is repeated for all tests, may be it makes sense to extract > > them out in @BeforeTest or another function. > > pavan kumar kolamuri wrote: > After every test case, we are cleaning up config store as it is required > for tests. We can put this code in one method and call that method , but in > one test case we are scheduling different workflow to check for succeded > case. Other test case we are scheduling more instances. Even though merging > those looking clumpsy so left it like that. > > Ajay Yadava wrote: > Will same approach as submitAndSchedule work? Explained in earlier comments. - pavan kumar ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40769/#review110174 ----------------------------------------------------------- On Dec. 14, 2015, 7:26 a.m., pavan kumar kolamuri wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40769/ > ----------------------------------------------------------- > > (Updated Dec. 14, 2015, 7:26 a.m.) > > > Review request for Falcon. > > > Bugs: https://issues.apache.org/jira/browse/FALCON-1447 > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/FALCON-1447 > > > Repository: falcon-git > > > Description > ------- > > For Native Scheduler we have tests where all services are mocked and even > jobs are not scheduled in oozie. Integration tests are needed for Native > Scheduler to run from end to end. > > > Diffs > ----- > > oozie/src/main/java/org/apache/oozie/client/LocalProxyOozieClient.java > f6e87c4 > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java > 2f97c0d > scheduler/pom.xml 336997d > > scheduler/src/main/java/org/apache/falcon/workflow/engine/FalconWorkflowEngine.java > ac7cde8 > > scheduler/src/test/java/org/apache/falcon/state/AbstractSchedulerTestBase.java > 48c1426 > unit/src/main/java/org/apache/falcon/unit/FalconUnitClient.java 9eb4277 > > unit/src/main/java/org/apache/falcon/unit/LocalSchedulableEntityManager.java > 0065c71 > unit/src/main/resources/oozie-site.xml 23d41eb > unit/src/test/java/org/apache/falcon/unit/FalconUnitTestBase.java 2a73516 > > unit/src/test/java/org/apache/falcon/unit/examples/JavaHelloWorldExample.java > PRE-CREATION > webapp/pom.xml 428f67e > > webapp/src/test/java/org/apache/falcon/resource/AbstractSchedulerManagerJerseyIT.java > PRE-CREATION > > webapp/src/test/java/org/apache/falcon/resource/EntitySchedulerManagerJerseyIT.java > PRE-CREATION > > webapp/src/test/java/org/apache/falcon/resource/InstanceSchedulerManagerJerseyIT.java > PRE-CREATION > > webapp/src/test/java/org/apache/falcon/resource/ProcessInstanceManagerIT.java > 6458b59 > webapp/src/test/java/org/apache/falcon/resource/UnitTestContext.java > 1d49353 > webapp/src/test/resources/helloworldworkflow.xml PRE-CREATION > webapp/src/test/resources/local-process-noinputs-template.xml PRE-CREATION > webapp/src/test/resources/runtime.properties 1da0ca7 > webapp/src/test/resources/startup.properties 756f315 > > Diff: https://reviews.apache.org/r/40769/diff/ > > > Testing > ------- > > These are test cases itself > > > Thanks, > > pavan kumar kolamuri > >
