You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Ajay Yadava <aj...@gmail.com> on 2015/09/17 09:52:59 UTC

Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

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

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 4ff8baa 
  common/src/main/java/org/apache/falcon/entity/FeedHelper.java 572923b 
  common/src/main/java/org/apache/falcon/entity/parser/CrossEntityValidations.java 2696552 
  common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java f22a343 
  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/NominalTimeBasedDelete.java PRE-CREATION 
  common/src/main/java/org/apache/falcon/service/LifecyclePolicyStore.java PRE-CREATION 
  common/src/main/java/org/apache/falcon/workflow/WorkflowEngineFactory.java 756c6b8 
  common/src/main/resources/startup.properties 39a412d 
  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 754fb06 
  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/NominalTimeBasedCoordinatorBuilder.java PRE-CREATION 
  lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/NominalTimeBasedDeleteBuilder.java PRE-CREATION 
  lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/NominalTimeBasedWorkflowBuilder.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 
  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 305ac36 

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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by pavan kumar kolamuri <pa...@gmail.com>.

> On Sept. 17, 2015, 7:13 p.m., pavan kumar kolamuri wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 431
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075838#file1075838line431>
> >
> >     Only one policy is returned. Something might missed here ?
> 
> Ajay Yadava wrote:
>     I am not sure if I understood your question completely but this is the correct behavior, nothing will get missed.

Looks like that method will return max only one policy ? is list is required as return type ?


> On Sept. 17, 2015, 7:13 p.m., pavan kumar kolamuri wrote:
> > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/OoziePolicyBuilderFactory.java, line 37
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075855#file1075855line37>
> >
> >     Should this static block in AbstractPolicyBuilderFactory as we are loading Policies irrespective of builderfactory right ? Or this is specific to OoziePolicyBuilderFactory ?
> 
> Ajay Yadava wrote:
>     I didn't completely understand your question but I checked again and this should be a static block and in right class.

Sorry for not making it clear , this property falcon.feed.lifecycle.policy.builders looks like common to all builders , should property name contains oozie also ?


- pavan kumar


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


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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Ajay Yadava <aj...@gmail.com>.

> On Sept. 17, 2015, 7:13 p.m., pavan kumar kolamuri wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 431
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075838#file1075838line431>
> >
> >     Only one policy is returned. Something might missed here ?

I am not sure if I understood your question completely but this is the correct behavior, nothing will get missed.


> On Sept. 17, 2015, 7:13 p.m., pavan kumar kolamuri wrote:
> > common/src/main/java/org/apache/falcon/service/LifecyclePolicyStore.java, line 69
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075846#file1075846line69>
> >
> >     minor nit can we clear the policyMap ?

Good suggestion, I have missed implementing it in this iteration. I will implement it along with the docs.


> On Sept. 17, 2015, 7:13 p.m., pavan kumar kolamuri wrote:
> > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/OoziePolicyBuilderFactory.java, line 37
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075855#file1075855line37>
> >
> >     Should this static block in AbstractPolicyBuilderFactory as we are loading Policies irrespective of builderfactory right ? Or this is specific to OoziePolicyBuilderFactory ?

I didn't completely understand your question but I checked again and this should be a static block and in right class.


> On Sept. 17, 2015, 7:13 p.m., pavan kumar kolamuri wrote:
> > pom.xml, line 851
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075867#file1075867line851>
> >
> >     space

It is same as above and looked more readable to me that way. I will immediately remove it if you feel otherwise. Let me know.


- Ajay


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


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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Ajay Yadava <aj...@gmail.com>.

> On Sept. 17, 2015, 7:13 p.m., pavan kumar kolamuri wrote:
> > common/src/main/java/org/apache/falcon/service/LifecyclePolicyStore.java, line 69
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075846#file1075846line69>
> >
> >     minor nit can we clear the policyMap ?
> 
> Ajay Yadava wrote:
>     Good suggestion, I have missed implementing it in this iteration. I will implement it along with the docs.

Fixed it in latest version.


> On Sept. 17, 2015, 7:13 p.m., pavan kumar kolamuri wrote:
> > pom.xml, line 851
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075867#file1075867line851>
> >
> >     space
> 
> Ajay Yadava wrote:
>     It is same as above and looked more readable to me that way. I will immediately remove it if you feel otherwise. Let me know.

I thought you were highlighting the blank line, now got what you meant. I have fixed it.


- Ajay


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


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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by pavan kumar kolamuri <pa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38450/#review99371
-----------------------------------------------------------



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

    Only one policy is returned. Something might missed here ?



common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 125)
<https://reviews.apache.org/r/38450/#comment156304>

    can be moved out of forloop



common/src/main/java/org/apache/falcon/service/LifecyclePolicyStore.java (line 69)
<https://reviews.apache.org/r/38450/#comment156313>

    minor nit can we clear the policyMap ?



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

    Should this static block in AbstractPolicyBuilderFactory as we are loading Policies irrespective of builderfactory right ? Or this is specific to OoziePolicyBuilderFactory ?



pom.xml (line 851)
<https://reviews.apache.org/r/38450/#comment156251>

    space


- pavan kumar kolamuri


On Sept. 17, 2015, 7:52 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38450/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2015, 7:52 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 4ff8baa 
>   common/src/main/java/org/apache/falcon/entity/FeedHelper.java 572923b 
>   common/src/main/java/org/apache/falcon/entity/parser/CrossEntityValidations.java 2696552 
>   common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java f22a343 
>   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/NominalTimeBasedDelete.java PRE-CREATION 
>   common/src/main/java/org/apache/falcon/service/LifecyclePolicyStore.java PRE-CREATION 
>   common/src/main/java/org/apache/falcon/workflow/WorkflowEngineFactory.java 756c6b8 
>   common/src/main/resources/startup.properties 39a412d 
>   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 754fb06 
>   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/NominalTimeBasedCoordinatorBuilder.java PRE-CREATION 
>   lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/NominalTimeBasedDeleteBuilder.java PRE-CREATION 
>   lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/NominalTimeBasedWorkflowBuilder.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 
>   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 305ac36 
> 
> 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
> 
>


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Ajay Yadava <aj...@gmail.com>.

> On Sept. 22, 2015, 11:46 a.m., Pallavi Rao wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 396
> > <https://reviews.apache.org/r/38450/diff/2/?file=1077636#file1077636line396>
> >
> >     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?

It's not required as of now as it's not being used for that currently.


> On Sept. 22, 2015, 11:46 a.m., Pallavi Rao wrote:
> > common/src/main/java/org/apache/falcon/lifecycle/retention/AgeBasedDelete.java, line 41
> > <https://reviews.apache.org/r/38450/diff/2/?file=1077642#file1077642line41>
> >
> >     Nit : policy.retention.agebaseddelete.limit seems more representative of the hierarchy.

this seems more natural to me as a user - retention policy agebaseddelete's limit :)


> On Sept. 22, 2015, 11:46 a.m., Pallavi Rao wrote:
> > common/src/main/java/org/apache/falcon/service/LifecyclePolicyMap.java, line 61
> > <https://reviews.apache.org/r/38450/diff/2/?file=1077644#file1077644line61>
> >
> >     Stage will be empty always.

Good catch. Fixed it.


> On Sept. 22, 2015, 11:46 a.m., Pallavi Rao wrote:
> > common/src/main/java/org/apache/falcon/service/LifecyclePolicyMap.java, line 58
> > <https://reviews.apache.org/r/38450/diff/2/?file=1077644#file1077644line58>
> >
> >     Feel adding the stage in the properties makes this user-friendly. For example,
> >     falcon.feed.lifecycle.retention.policies or falcon.feed.lifecycle.replication.policies.

It might read better but it won't be user friendly. Instead of one property there will be many properties and we will need to add validations that the properties in each are belonging to correct stage. Also, with every new stage added to lifecycle this code will need to be changed.


> On Sept. 22, 2015, 11:46 a.m., Pallavi Rao wrote:
> > common/src/main/java/org/apache/falcon/workflow/WorkflowEngineFactory.java, line 34
> > <https://reviews.apache.org/r/38450/diff/2/?file=1077645#file1077645line34>
> >
> >     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).

Since we are discussing it in JIRA, I am dropping it from here, will reply on JIRA to avoid duplicate effort :)


> On Sept. 22, 2015, 11:46 a.m., Pallavi Rao wrote:
> > common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java, line 178
> > <https://reviews.apache.org/r/38450/diff/2/?file=1077649#file1077649line178>
> >
> >     Additional test cases can be added to check for validation against sla and late cut off.. Not necessarily in this class.

I have already added them in AgeBasedDeleteTest, looks like you missed it :)


> On Sept. 22, 2015, 11:46 a.m., Pallavi Rao wrote:
> > client/src/main/resources/feed-0.1.xsd, line 128
> > <https://reviews.apache.org/r/38450/diff/2/?file=1077635#file1077635line128>
> >
> >     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.

Documented the behavior.


> On Sept. 22, 2015, 11:46 a.m., Pallavi Rao wrote:
> > common/src/main/resources/startup.properties, line 48
> > <https://reviews.apache.org/r/38450/diff/2/?file=1077646#file1077646line48>
> >
> >     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.

I prefer it this way as this is a sort of poor man's DI. Moreover, it is not a big deal as a user will need to rarely specify it.


- Ajay


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


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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Pallavi Rao <pa...@inmobi.com>.
-----------------------------------------------------------
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
> 
>


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Ajay Yadava <aj...@gmail.com>.

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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38450/#review99804
-----------------------------------------------------------



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

    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>



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

    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.



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

    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?



common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 98)
<https://reviews.apache.org/r/38450/#comment156772>

    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.


- Balu Vellanki


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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Ajay Yadava <aj...@gmail.com>.

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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Srikanth Sundarrajan <sr...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38450/#review99990
-----------------------------------------------------------



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

    Should this be a FeedHelper util function ?



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

    How will work when there are multiple stages? Would we have to modify this piece of code for each new stage being added ?



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

    Find and replace occurences of nominalTime



lifecycle/pom.xml (line 101)
<https://reviews.apache.org/r/38450/#comment157060>

    Do we need the oozie-schemas to be present again inside the lifecycle module ?



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

    If builders are empty ?



lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java (line 158)
<https://reviews.apache.org/r/38450/#comment157062>

    Do we want to hidethis behind private scope ?



pom.xml (line 418)
<https://reviews.apache.org/r/38450/#comment157054>

    Check indentation


- Srikanth Sundarrajan


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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38450/#review99809
-----------------------------------------------------------


Apart from minor comments, the patch looks good. Applies and builds cleanly.

- Balu Vellanki


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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Srikanth Sundarrajan <sr...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38450/#review100814
-----------------------------------------------------------

Ship it!


Ship It!

- Srikanth Sundarrajan


On Sept. 27, 2015, 10:56 p.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:56 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 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
> 
>


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38450/
-----------------------------------------------------------

(Updated Sept. 27, 2015, 10:56 p.m.)


Review request for Falcon.


Changes
-------

Added execution order and timeout in the coordinator.


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 (updated)
-----

  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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
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 (updated)
-----

  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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
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 (updated)
-----

  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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.

> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 402
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075838#file1075838line402>
> >
> >     "limit" is the key attribute for retention. It does not seem good to keep limit in key-value property section. It should be element in retention-stage.
> 
> Ajay Yadava wrote:
>     Limit has to be a generic type because not all policies will have same type of retention limit e.g. size based delete and instance time based delete have two different types of limits.

I agree with you that retention limit will be of different type and it has to be generic type. But I am of the opinion that "limit" should be element instead of key-value property and for limit element size/time can be retention basis attributes.


- Peeyush


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


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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.

> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > lifecycle/pom.xml, line 117
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075854#file1075854line117>
> >
> >     Can we use latest Oozie coordinator xsd version.

We can bump the coordinator xsd version when we bump the Falcon feed/process xsd.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > lifecycle/pom.xml, line 136
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075854#file1075854line136>
> >
> >     Can we use latest Oozie workflow xsd version.

We can bump the workflow xsd version when we bump the Falcon feed/process xsd.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > lifecycle/pom.xml, line 154
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075854#file1075854line154>
> >
> >     can we use latest hive action xsd version.

We can bump the hive xsd version when we bump the Falcon feed/process xsd.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > lifecycle/pom.xml, line 173
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075854#file1075854line173>
> >
> >     Can we use latest bundle xsd version.

We can bump the bundle xsd version when we bump the Falcon feed/process xsd.


- Peeyush


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


On Sept. 17, 2015, 7:52 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38450/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2015, 7:52 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 4ff8baa 
>   common/src/main/java/org/apache/falcon/entity/FeedHelper.java 572923b 
>   common/src/main/java/org/apache/falcon/entity/parser/CrossEntityValidations.java 2696552 
>   common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java f22a343 
>   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/NominalTimeBasedDelete.java PRE-CREATION 
>   common/src/main/java/org/apache/falcon/service/LifecyclePolicyStore.java PRE-CREATION 
>   common/src/main/java/org/apache/falcon/workflow/WorkflowEngineFactory.java 756c6b8 
>   common/src/main/resources/startup.properties 39a412d 
>   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 754fb06 
>   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/NominalTimeBasedCoordinatorBuilder.java PRE-CREATION 
>   lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/NominalTimeBasedDeleteBuilder.java PRE-CREATION 
>   lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/NominalTimeBasedWorkflowBuilder.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 
>   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 305ac36 
> 
> 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
> 
>


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Ajay Yadava <aj...@gmail.com>.

> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java, line 434
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075859#file1075859line434>
> >
> >     Can we move this function to class HiveUtil.

It is not useful outside of OozieBuilder, so it makes more sense to keep it here.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > client/src/main/resources/feed-0.1.xsd, line 120
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075837#file1075837line120>
> >
> >     Can you add details about lifecycle in xsd doc section.

Details about lifecycle are already present at the position where element is defined.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > client/src/main/resources/feed-0.1.xsd, line 150
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075837#file1075837line150>
> >
> >     As we are defining retention through lifecycle, do we require older retention specifically. Is this required for backward compatibility, please confirm.

You are right, this is requried for backward compatibility.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 402
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075838#file1075838line402>
> >
> >     "limit" is the key attribute for retention. It does not seem good to keep limit in key-value property section. It should be element in retention-stage.

Limit has to be a generic type because not all policies will have same type of retention limit e.g. size based delete and instance time based delete have two different types of limits.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 429
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075838#file1075838line429>
> >
> >     Returning DefaultPolicyName does not make sense. If user want to use retention stage, they must specify the required policy.

It is an optional element on lines of Convention over Configuration.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, line 128
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075840#file1075840line128>
> >
> >     What is the requirement to define two type of retention in entity global and cluster. From user experience perspective, how this will help the user. Can we think to make it as one like currently.

Making it one will not solve cases where we need different retention behavior in different clusters. It is an improvement over the existing behavior. Other feeds like validity, sla etc. can also be overridden at cluster level.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java, line 393
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075859#file1075859line393>
> >
> >     Can we move this function to class HiveUtil.

It is not useful outside of OozieBuilder, so it makes more sense to keep it here.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java, line 359
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075859#file1075859line359>
> >
> >     Can we move this function to class HiveUtil.

It is not useful outside of OozieBuilder, so it makes more sense to keep it here.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java, line 301
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075859#file1075859line301>
> >
> >     What about support for other action like FS,SSH,Sqoop .

How are actions like ssh and sqoop relevant in retention?


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/NominalTimeBasedWorkflowBuilder.java, line 119
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075858#file1075858line119>
> >
> >     Can we move this function to class HiveUtil.

It is "workflow" specific and Hive doesn't understand workflow, hence it is better to be put here.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > common/src/main/java/org/apache/falcon/lifecycle/retention/NominalTimeBasedDelete.java, line 60
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075845#file1075845line60>
> >
> >     "limit" is the key attribute for retention. It does not seem good to keep limit in key-value property section. It should be element in retention-stage.

Limit has to be a generic type because not all policies will have same type of retention limit e.g. size based delete and instance time based delete have two different types of limits.


> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > src/conf/startup.properties, line 55
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075868#file1075868line55>
> >
> >     Can we make the lifecycle policy to be defined like: *.falcon.feed.lifecycle.policies=org.apache.falcon.lifecycle.retention.NominalTimeBasedDelete#NominalTimeBasedDelete

This seems to be a break from current convention. What benefits this has over current scheme?


- Ajay


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


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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Ajay Yadava <aj...@gmail.com>.

> On Sept. 18, 2015, 10:35 a.m., Peeyush Bishnoi wrote:
> > lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java, line 294
> > <https://reviews.apache.org/r/38450/diff/1/?file=1075859#file1075859line294>
> >
> >     For each inner iteration, you are setting local List variable to null and adding jar files. With this all jar files will not be added from multiple actions. Can you check this.

Only one needs to be loaded and it will work correctly, variable is reassigned.


- Ajay


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


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


Re: Review Request 38450: FALCON-965 Open up Feed lifecycle for user extension

Posted by Peeyush Bishnoi <bp...@yahoo.co.in>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38450/#review99507
-----------------------------------------------------------



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

    Can you add details about lifecycle in xsd doc section.



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

    As we are defining retention through lifecycle, do we require older retention specifically. Is this required for backward compatibility, please confirm.



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

    "limit" is the key attribute for retention. It does not seem good to keep limit in key-value property section. It should be element in retention-stage.



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

    Returning DefaultPolicyName does not make sense. If user want to use retention stage, they must specify the required policy.



common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 126)
<https://reviews.apache.org/r/38450/#comment156406>

    What is the requirement to define two type of retention in entity global and cluster. From user experience perspective, how this will help the user. Can we think to make it as one like currently.



common/src/main/java/org/apache/falcon/lifecycle/retention/NominalTimeBasedDelete.java (line 60)
<https://reviews.apache.org/r/38450/#comment156412>

    "limit" is the key attribute for retention. It does not seem good to keep limit in key-value property section. It should be element in retention-stage.



lifecycle/pom.xml (line 117)
<https://reviews.apache.org/r/38450/#comment156407>

    Can we use latest Oozie coordinator xsd version.



lifecycle/pom.xml (line 136)
<https://reviews.apache.org/r/38450/#comment156408>

    Can we use latest Oozie workflow xsd version.



lifecycle/pom.xml (line 154)
<https://reviews.apache.org/r/38450/#comment156409>

    can we use latest hive action xsd version.



lifecycle/pom.xml (line 173)
<https://reviews.apache.org/r/38450/#comment156410>

    Can we use latest bundle xsd version.



lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/NominalTimeBasedWorkflowBuilder.java (line 119)
<https://reviews.apache.org/r/38450/#comment156414>

    Can we move this function to class HiveUtil.



lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java (line 294)
<https://reviews.apache.org/r/38450/#comment156427>

    For each inner iteration, you are setting local List variable to null and adding jar files. With this all jar files will not be added from multiple actions. Can you check this.



lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java (line 301)
<https://reviews.apache.org/r/38450/#comment156425>

    What about support for other action like FS,SSH,Sqoop .



lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java (line 359)
<https://reviews.apache.org/r/38450/#comment156415>

    Can we move this function to class HiveUtil.



lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java (line 393)
<https://reviews.apache.org/r/38450/#comment156416>

    Can we move this function to class HiveUtil.



lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/utils/OozieBuilderUtils.java (line 434)
<https://reviews.apache.org/r/38450/#comment156417>

    Can we move this function to class HiveUtil.



src/conf/startup.properties (line 55)
<https://reviews.apache.org/r/38450/#comment156420>

    Can we make the lifecycle policy to be defined like: *.falcon.feed.lifecycle.policies=org.apache.falcon.lifecycle.retention.NominalTimeBasedDelete#NominalTimeBasedDelete


- Peeyush Bishnoi


On Sept. 17, 2015, 7:52 a.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38450/
> -----------------------------------------------------------
> 
> (Updated Sept. 17, 2015, 7:52 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 4ff8baa 
>   common/src/main/java/org/apache/falcon/entity/FeedHelper.java 572923b 
>   common/src/main/java/org/apache/falcon/entity/parser/CrossEntityValidations.java 2696552 
>   common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java f22a343 
>   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/NominalTimeBasedDelete.java PRE-CREATION 
>   common/src/main/java/org/apache/falcon/service/LifecyclePolicyStore.java PRE-CREATION 
>   common/src/main/java/org/apache/falcon/workflow/WorkflowEngineFactory.java 756c6b8 
>   common/src/main/resources/startup.properties 39a412d 
>   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 754fb06 
>   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/NominalTimeBasedCoordinatorBuilder.java PRE-CREATION 
>   lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/NominalTimeBasedDeleteBuilder.java PRE-CREATION 
>   lifecycle/src/main/java/org/apache/falcon/lifecycle/engine/oozie/retention/NominalTimeBasedWorkflowBuilder.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 
>   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 305ac36 
> 
> 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
> 
>