falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ajay Yadava" <ajayn...@gmail.com>
Subject Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension
Date Sun, 27 Sep 2015 10:40:50 GMT


> 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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message