falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Pallavi Rao" <pallavi....@inmobi.com>
Subject Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension
Date Tue, 22 Sep 2015 11:46:02 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38450/#review99967
-----------------------------------------------------------



client/src/main/resources/feed-0.1.xsd (line 128)
<https://reviews.apache.org/r/38450/#comment157024>

    Since we are not deprecating the old retention element. Do we throw an error if both are
defined? Or are we just saying lifecycle takes precedence? Either way this needs to be called
out.



common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 395)
<https://reviews.apache.org/r/38450/#comment157018>

    My understanding was that there are multiple policies per stage. So, is this method intended
to return all policies across stages for a feed? In this case, it might useful to return a
Map<Stage, String>. If not, should Stage be one of the input params?



common/src/main/java/org/apache/falcon/lifecycle/retention/AgeBasedDelete.java (line 41)
<https://reviews.apache.org/r/38450/#comment157032>

    Nit : policy.retention.agebaseddelete.limit seems more representative of the hierarchy.



common/src/main/java/org/apache/falcon/service/LifecyclePolicyMap.java (line 58)
<https://reviews.apache.org/r/38450/#comment157026>

    Feel adding the stage in the properties makes this user-friendly. For example,
    falcon.feed.lifecycle.retention.policies or falcon.feed.lifecycle.replication.policies.



common/src/main/java/org/apache/falcon/service/LifecyclePolicyMap.java (line 61)
<https://reviews.apache.org/r/38450/#comment157027>

    Stage will be empty always.



common/src/main/java/org/apache/falcon/workflow/WorkflowEngineFactory.java (line 34)
<https://reviews.apache.org/r/38450/#comment157029>

    Lifecycle is not really a Workflow Engine. Rather, a workflow engine should understand
Lifecycle like it understands Entity (Feed and Process) now. This makes it difficult to:
    1. Plugging in other workflow engines (such as our native scheduler)
    2. This is meant to be extensible by user. We don't users to be building co-ords, bundles.
It can be more like, the builder returns the user-workflow to execute (and properties).



common/src/main/resources/startup.properties (line 48)
<https://reviews.apache.org/r/38450/#comment157030>

    Can't we have a getBuilder() as method in LifeCyclePolicy and have the method return an
appropriate builder, rather than user having to specify it here.



common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java (line 177)
<https://reviews.apache.org/r/38450/#comment157033>

    Additional test cases can be added to check for validation against sla and late cut off..
Not necessarily in this class.



lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/OoziePolicyBuilderFactory.java
(line 38)
<https://reviews.apache.org/r/38450/#comment157031>

    See comment above, we can have the Policy return the appropriate builder?


- Pallavi Rao


On Sept. 20, 2015, 3:40 p.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38450/
> -----------------------------------------------------------
> 
> (Updated Sept. 20, 2015, 3:40 p.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 992fc51

>   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 6179855 
>   common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java c70cfcc 
>   common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 1e9b72f

>   common/src/test/resources/config/feed/feed-0.3.xml PRE-CREATION 
>   common/src/test/resources/config/feed/feed-0.4.xml PRE-CREATION 
>   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