----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35452/#review88027 -----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/EntityUtil.java (line 811) <https://reviews.apache.org/r/35452/#comment140434> Assert that referenceTime is > startTime, if lesser getInstanceSequence returns -1 common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java (line 183) <https://reviews.apache.org/r/35452/#comment142117> Does it make sense to assert on the actual values besides the size of the set. Also see if other such asserts introduced by the patch can be changed. prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java (line 392) <https://reviews.apache.org/r/35452/#comment142116> Though not an issue caused by this patch, there seems to be no logging of the actual exception. It would be useful to log the full exception since we aren't passing it up. - Srikanth Sundarrajan On June 15, 2015, 10:57 a.m., Ajay Yadava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35452/ > ----------------------------------------------------------- > > (Updated June 15, 2015, 10:57 a.m.) > > > Review request for Falcon and Srikanth Sundarrajan. > > > Bugs: FALCON-1260 > https://issues.apache.org/jira/browse/FALCON-1260 > > > Repository: falcon-git > > > Description > ------- > > There were some bugs in the current implementation. > > > Diffs > ----- > > > client/src/main/java/org/apache/falcon/resource/SchedulableEntityInstance.java > 2a7ecdb > common/src/main/java/org/apache/falcon/entity/EntityUtil.java 7ebf39e > common/src/main/java/org/apache/falcon/entity/FeedHelper.java 9f4eb61 > common/src/main/java/org/apache/falcon/entity/ProcessHelper.java fe78bc8 > common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java f70edfb > common/src/test/java/org/apache/falcon/entity/ProcessHelperTest.java > 0d396ae > docs/src/site/twiki/FalconCLI.twiki 50dce84 > prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java > 72f9fe4 > > prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java > 1a8396c > > Diff: https://reviews.apache.org/r/35452/diff/ > > > Testing > ------- > > Unit tests added for all the issues and tests were also added for newer > scenarios. > > > Thanks, > > Ajay Yadava > >
