> On Sept. 22, 2015, 3:28 p.m., Srikanth Sundarrajan wrote: > > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 381 > > <https://reviews.apache.org/r/38450/diff/2/?file=1077636#file1077636line381> > > > > Should this be a FeedHelper util function ?
Yes. That will be much better. Added it. > On Sept. 22, 2015, 3:28 p.m., Srikanth Sundarrajan wrote: > > common/src/main/java/org/apache/falcon/lifecycle/retention/AgeBasedDelete.java, > > line 36 > > <https://reviews.apache.org/r/38450/diff/2/?file=1077642#file1077642line36> > > > > Find and replace occurences of nominalTime Done. This had somehow escaped :) > On Sept. 22, 2015, 3:28 p.m., Srikanth Sundarrajan wrote: > > lifecycle/pom.xml, line 101 > > <https://reviews.apache.org/r/38450/diff/2/?file=1077652#file1077652line101> > > > > Do we need the oozie-schemas to be present again inside the lifecycle > > module ? Yes. They are required because they are used in building coordinators and workflow. > On Sept. 22, 2015, 3:28 p.m., Srikanth Sundarrajan wrote: > > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java, > > line 158 > > <https://reviews.apache.org/r/38450/diff/2/?file=1077657#file1077657line158> > > > > Do we want to hidethis behind private scope ? Done. > On Sept. 22, 2015, 3:28 p.m., Srikanth Sundarrajan wrote: > > pom.xml, line 418 > > <https://reviews.apache.org/r/38450/diff/2/?file=1077666#file1077666line418> > > > > Check indentation fixed it. > On Sept. 22, 2015, 3:28 p.m., Srikanth Sundarrajan wrote: > > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/OoziePolicyBuilderFactory.java, > > line 38 > > <https://reviews.apache.org/r/38450/diff/2/?file=1077653#file1077653line38> > > > > If builders are empty ? Fixed it. > On Sept. 22, 2015, 3:28 p.m., Srikanth Sundarrajan wrote: > > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 400 > > <https://reviews.apache.org/r/38450/diff/2/?file=1077636#file1077636line400> > > > > How will work when there are multiple stages? Would we have to modify > > this piece of code for each new stage being added ? Yes, we will need to modify it. Unfortunately, there is not a cleaner alternative to iterate over all the lifecycles of a feed (even with using extensions in xsd) - Ajay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38450/#review99990 ----------------------------------------------------------- On Sept. 27, 2015, 10:39 a.m., Ajay Yadava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38450/ > ----------------------------------------------------------- > > (Updated Sept. 27, 2015, 10:39 a.m.) > > > Review request for Falcon. > > > Bugs: FALCON-965 > https://issues.apache.org/jira/browse/FALCON-965 > > > Repository: falcon-git > > > Description > ------- > > For details on feed lifecycle feature and motivation behind it, please refer > FALCON-965. This is the base framework for lifecycle with a feature parity of > retention stage as a reference implementation. > > > Diffs > ----- > > client/src/main/resources/feed-0.1.xsd 2af28d2 > common/src/main/java/org/apache/falcon/entity/FeedHelper.java 572923b > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java > 4f5599e > > common/src/main/java/org/apache/falcon/lifecycle/AbstractPolicyBuilderFactory.java > PRE-CREATION > common/src/main/java/org/apache/falcon/lifecycle/FeedLifecycleStage.java > PRE-CREATION > common/src/main/java/org/apache/falcon/lifecycle/LifecyclePolicy.java > PRE-CREATION > common/src/main/java/org/apache/falcon/lifecycle/PolicyBuilder.java > PRE-CREATION > > common/src/main/java/org/apache/falcon/lifecycle/retention/AgeBasedDelete.java > PRE-CREATION > > common/src/main/java/org/apache/falcon/lifecycle/retention/RetentionPolicy.java > PRE-CREATION > common/src/main/java/org/apache/falcon/service/LifecyclePolicyMap.java > PRE-CREATION > common/src/main/java/org/apache/falcon/workflow/WorkflowEngineFactory.java > 756c6b8 > common/src/main/resources/startup.properties 9db460c > common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java e9946c4 > common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java c70cfcc > > common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java > b6fdb13 > common/src/test/resources/config/feed/feed-0.3.xml PRE-CREATION > common/src/test/resources/config/feed/feed-0.4.xml PRE-CREATION > docs/src/site/twiki/EntitySpecification.twiki d4f4140 > lifecycle/pom.xml PRE-CREATION > > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/OoziePolicyBuilderFactory.java > PRE-CREATION > > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/AgeBasedCoordinatorBuilder.java > PRE-CREATION > > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/AgeBasedDeleteBuilder.java > PRE-CREATION > > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/AgeBasedWorkflowBuilder.java > PRE-CREATION > > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java > PRE-CREATION > lifecycle/src/main/resources/action/feed/eviction-action.xml PRE-CREATION > lifecycle/src/main/resources/binding/jaxb-binding.xjb PRE-CREATION > > lifecycle/src/test/java/org/apache/falcon/lifecycle/retention/AgeBasedDeleteTest.java > PRE-CREATION > oozie/pom.xml 157edf9 > oozie/src/main/java/org/apache/falcon/oozie/feed/FeedBundleBuilder.java > b819dee > > oozie/src/test/java/org/apache/falcon/oozie/feed/OozieFeedWorkflowBuilderTest.java > 7d0174a > oozie/src/test/resources/feed/fs-retention-feed.xml PRE-CREATION > oozie/src/test/resources/feed/fs-retention-lifecycle-feed.xml PRE-CREATION > pom.xml 646de69 > src/conf/startup.properties 8f3bc35 > > Diff: https://reviews.apache.org/r/38450/diff/ > > > Testing > ------- > > Unit tests have been added for all the methods. > I have compared the new artifacts(workflow, bundle, config-default and > coordinator xmls) with the old ones. > > > Thanks, > > Ajay Yadava > >
