> On Oct. 28, 2015, 6:35 p.m., Sowmya Ramesh wrote: > > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 808 > > <https://reviews.apache.org/r/39711/diff/4/?file=1111629#file1111629line808> > > > > I don't fully understand the fix here. If retention stage is not > > defined for lifecycle shouldn't it fallback to old retention policy? > > > > Retetion policy frequency was not validated before. Now, with your code > > change retention validation happens even for those cases isn't if user > > defnes lifecycle without retention stage. Isn't it breaking the old > > behavior. > > > > If retention stage is not defined even when lifecycle is present then > > it shoudl fall back to old behavior. So validation should be skipped and > > retention should be set as in FeedRetentionCoordinatorBuilder. Am I missing > > something here?
validateRetentionFrequency is only used for validating Lifecycle related retention policy so it wont affect the old retention behaviour. - PRAGYA ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39711/#review104322 ----------------------------------------------------------- On Oct. 28, 2015, 6:04 p.m., Ajay Yadava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39711/ > ----------------------------------------------------------- > > (Updated Oct. 28, 2015, 6:04 p.m.) > > > Review request for Falcon. > > > Bugs: FALCON-1560 > https://issues.apache.org/jira/browse/FALCON-1560 > > > Repository: falcon-git > > > Description > ------- > > Lifecycle does not allow feed with frequency greater than days(1) > > > Diffs > ----- > > common/src/main/java/org/apache/falcon/entity/FeedHelper.java 5c252a8 > common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 4020d36 > > common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java > 905be68 > > Diff: https://reviews.apache.org/r/39711/diff/ > > > Testing > ------- > > Added unit test for the scenarios. > > > Thanks, > > Ajay Yadava > >
