> On Sept. 21, 2015, 6:41 p.m., Balu Vellanki wrote: > > client/src/main/resources/feed-0.1.xsd, line 469 > > <https://reviews.apache.org/r/38450/diff/2/?file=1077635#file1077635line469> > > > > Why is maxOccurs set to 1? It is possible that users need more than one > > property. If the only porperty allowed is > > "retention.policy.agebaseddelete.limit", it makes more sense to have it as > > <xs:element type="xs:string" > > name="retention.policy.agebaseddelete.limit" minOccurs="0" > > maxOccurs="1"></xs:element>
It is the "properties" element and not the "property" element. Multiple properties are allowed. > On Sept. 21, 2015, 6:41 p.m., Balu Vellanki wrote: > > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 399 > > <https://reviews.apache.org/r/38450/diff/2/?file=1077636#file1077636line399> > > > > Minor nit : I understand getPolicies() will never be called for invalid > > cluster, but not having a check to make sure cluster is valid for this feed > > can cause future problems. > > if (cluster !=null) {} -- Similar to the method above. Good catch. Added it. > On Sept. 21, 2015, 6:41 p.m., Balu Vellanki wrote: > > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 405 > > <https://reviews.apache.org/r/38450/diff/2/?file=1077636#file1077636line405> > > > > I see that the getPolicies currently returns 0 or 1 policies. But > > results is "List<String>", is this to support adding more policies in > > future? Exactly :) > On Sept. 21, 2015, 6:41 p.m., Balu Vellanki wrote: > > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, > > line 102 > > <https://reviews.apache.org/r/38450/diff/2/?file=1077637#file1077637line102> > > > > This is not from your code. But there seems to be two validateACL(feed) > > calls. This can be fixed if you are creating another patch. If not, please > > ignore this comment. Sharp :) fixed it! - Ajay ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38450/#review99804 ----------------------------------------------------------- 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 > >
