You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by sh...@inmobi.com on 2014/03/17 10:45:22 UTC
Review Request 19286: FALCON-356 Merge OozieProcessMapper and
OozieProcessWorkflowBuilder
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19286/
-----------------------------------------------------------
Review request for Falcon.
Repository: falcon-git
Description
-------
There is one to one mapping between OozieProcessMapper and OozieProcessWorkflowBuilder. Entity to oozie workflow mapping is spread across these two classes. Same goes for OozieFeedMapper and OozieFeedWorkflowBuilder.
Diffs
-----
common/src/main/java/org/apache/falcon/util/ReflectionUtils.java 4a00fa9
common/src/main/java/org/apache/falcon/workflow/WorkflowBuilder.java 26243e7
feed/src/main/java/org/apache/falcon/converter/OozieFeedMapper.java 2b3315f
feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java 5e3a30e
feed/src/test/java/org/apache/falcon/converter/OozieFeedMapperTest.java e610df2
feed/src/test/java/org/apache/falcon/converter/OozieFeedWorkflowBuilderTest.java PRE-CREATION
oozie/src/main/java/org/apache/falcon/converter/AbstractOozieEntityMapper.java f443939
oozie/src/main/java/org/apache/falcon/util/OozieUtils.java 2f53370
oozie/src/main/java/org/apache/falcon/workflow/OozieWorkflowBuilder.java e5a01ca
oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java ac8862e
process/src/main/java/org/apache/falcon/converter/OozieProcessMapper.java e638961
process/src/main/java/org/apache/falcon/workflow/OozieProcessWorkflowBuilder.java 4e5e8c6
process/src/test/java/org/apache/falcon/converter/OozieProcessMapperLateProcessTest.java fbda0ea
process/src/test/java/org/apache/falcon/converter/OozieProcessMapperTest.java 22bf9fe
process/src/test/java/org/apache/falcon/converter/OozieProcessWorkflowBuilderTest.java PRE-CREATION
retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java 1e7cc04
Diff: https://reviews.apache.org/r/19286/diff/
Testing
-------
UTs
Thanks,
shwethags
Re: Review Request 19286: FALCON-356 Merge OozieProcessMapper and
OozieProcessWorkflowBuilder
Posted by sh...@inmobi.com.
> On March 18, 2014, 5:48 a.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/util/ReflectionUtils.java, line 41
> > <https://reviews.apache.org/r/19286/diff/1/?file=522468#file522468line41>
> >
> > Should constructor args be var-args instead ?
Can be. Didn't have any usecase. Changing now
> On March 18, 2014, 5:48 a.m., Srikanth Sundarrajan wrote:
> > common/src/main/java/org/apache/falcon/util/ReflectionUtils.java, line 69
> > <https://reviews.apache.org/r/19286/diff/1/?file=522468#file522468line69>
> >
> > What if arg is null ?
if arg is null, there is no way to figure out the class. In method doc, mentioned that args can't be null
> On March 18, 2014, 5:48 a.m., Srikanth Sundarrajan wrote:
> > feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java, line 97
> > <https://reviews.apache.org/r/19286/diff/1/?file=522471#file522471line97>
> >
> > Why does the log say process validity ? Isn't this feed workflow builder ?
yes, it is. copied the comment from process. fixing
> On March 18, 2014, 5:48 a.m., Srikanth Sundarrajan wrote:
> > feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java, line 139
> > <https://reviews.apache.org/r/19286/diff/1/?file=522471#file522471line139>
> >
> > Dont we need to check for replicationCoord being null ?
Retention coord can be null as retention is scheduled with start time as now. But replication coord is scheduled with the given start time and can't be null
> On March 18, 2014, 5:48 a.m., Srikanth Sundarrajan wrote:
> > feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java, line 167
> > <https://reviews.apache.org/r/19286/diff/1/?file=522471#file522471line167>
> >
> > Why are the retention mapper & replication mapper returning coord / workflows etc. Can we keep their behavior consistent, it is quite useful in following the code
I just copied all the code from OozieFeedMapper. Looks like RetentionOozieWorkflowMapper and ReplicationOozieWorkflowMapper don't have a common interface. We can fix that later
- shwethags
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19286/#review37535
-----------------------------------------------------------
On March 17, 2014, 9:50 a.m., shwethags wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19286/
> -----------------------------------------------------------
>
> (Updated March 17, 2014, 9:50 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> There is one to one mapping between OozieProcessMapper and OozieProcessWorkflowBuilder. Entity to oozie workflow mapping is spread across these two classes. Same goes for OozieFeedMapper and OozieFeedWorkflowBuilder.
>
> modified: common/src/main/java/org/apache/falcon/util/ReflectionUtils.java
> modified: common/src/main/java/org/apache/falcon/workflow/WorkflowBuilder.java
> renamed: feed/src/main/java/org/apache/falcon/converter/OozieFeedMapper.java -> feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java
> renamed: feed/src/test/java/org/apache/falcon/converter/OozieFeedMapperTest.java -> feed/src/test/java/org/apache/falcon/converter/OozieFeedWorkflowBuilderTest.java
> modified: oozie/src/main/java/org/apache/falcon/util/OozieUtils.java
> renamed: oozie/src/main/java/org/apache/falcon/converter/AbstractOozieEntityMapper.java -> oozie/src/main/java/org/apache/falcon/workflow/OozieWorkflowBuilder.java
> modified: oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java
> renamed: process/src/main/java/org/apache/falcon/converter/OozieProcessMapper.java -> process/src/main/java/org/apache/falcon/workflow/OozieProcessWorkflowBuilder.java
> deleted: process/src/test/java/org/apache/falcon/converter/OozieProcessMapperLateProcessTest.java
> renamed: process/src/test/java/org/apache/falcon/converter/OozieProcessMapperTest.java -> process/src/test/java/org/apache/falcon/converter/OozieProcessWorkflowBuilderTest.java
> modified: retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/util/ReflectionUtils.java 4a00fa9
> common/src/main/java/org/apache/falcon/workflow/WorkflowBuilder.java 26243e7
> feed/src/main/java/org/apache/falcon/converter/OozieFeedMapper.java 2b3315f
> feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java 5e3a30e
> feed/src/test/java/org/apache/falcon/converter/OozieFeedMapperTest.java e610df2
> feed/src/test/java/org/apache/falcon/converter/OozieFeedWorkflowBuilderTest.java PRE-CREATION
> oozie/src/main/java/org/apache/falcon/converter/AbstractOozieEntityMapper.java f443939
> oozie/src/main/java/org/apache/falcon/util/OozieUtils.java 2f53370
> oozie/src/main/java/org/apache/falcon/workflow/OozieWorkflowBuilder.java e5a01ca
> oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java ac8862e
> process/src/main/java/org/apache/falcon/converter/OozieProcessMapper.java e638961
> process/src/main/java/org/apache/falcon/workflow/OozieProcessWorkflowBuilder.java 4e5e8c6
> process/src/test/java/org/apache/falcon/converter/OozieProcessMapperLateProcessTest.java fbda0ea
> process/src/test/java/org/apache/falcon/converter/OozieProcessMapperTest.java 22bf9fe
> process/src/test/java/org/apache/falcon/converter/OozieProcessWorkflowBuilderTest.java PRE-CREATION
> retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java 1e7cc04
>
> Diff: https://reviews.apache.org/r/19286/diff/
>
>
> Testing
> -------
>
> UTs
>
>
> Thanks,
>
> shwethags
>
>
Re: Review Request 19286: FALCON-356 Merge OozieProcessMapper and
OozieProcessWorkflowBuilder
Posted by Srikanth Sundarrajan <sr...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19286/#review37535
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/util/ReflectionUtils.java
<https://reviews.apache.org/r/19286/#comment69089>
Should constructor args be var-args instead ?
common/src/main/java/org/apache/falcon/util/ReflectionUtils.java
<https://reviews.apache.org/r/19286/#comment69090>
What if arg is null ?
feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java
<https://reviews.apache.org/r/19286/#comment69091>
Why does the log say process validity ? Isn't this feed workflow builder ?
feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java
<https://reviews.apache.org/r/19286/#comment69093>
Dont we need to check for replicationCoord being null ?
feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java
<https://reviews.apache.org/r/19286/#comment69094>
Why are the retention mapper & replication mapper returning coord / workflows etc. Can we keep their behavior consistent, it is quite useful in following the code
- Srikanth Sundarrajan
On March 17, 2014, 9:50 a.m., shwethags wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19286/
> -----------------------------------------------------------
>
> (Updated March 17, 2014, 9:50 a.m.)
>
>
> Review request for Falcon.
>
>
> Repository: falcon-git
>
>
> Description
> -------
>
> There is one to one mapping between OozieProcessMapper and OozieProcessWorkflowBuilder. Entity to oozie workflow mapping is spread across these two classes. Same goes for OozieFeedMapper and OozieFeedWorkflowBuilder.
>
> modified: common/src/main/java/org/apache/falcon/util/ReflectionUtils.java
> modified: common/src/main/java/org/apache/falcon/workflow/WorkflowBuilder.java
> renamed: feed/src/main/java/org/apache/falcon/converter/OozieFeedMapper.java -> feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java
> renamed: feed/src/test/java/org/apache/falcon/converter/OozieFeedMapperTest.java -> feed/src/test/java/org/apache/falcon/converter/OozieFeedWorkflowBuilderTest.java
> modified: oozie/src/main/java/org/apache/falcon/util/OozieUtils.java
> renamed: oozie/src/main/java/org/apache/falcon/converter/AbstractOozieEntityMapper.java -> oozie/src/main/java/org/apache/falcon/workflow/OozieWorkflowBuilder.java
> modified: oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java
> renamed: process/src/main/java/org/apache/falcon/converter/OozieProcessMapper.java -> process/src/main/java/org/apache/falcon/workflow/OozieProcessWorkflowBuilder.java
> deleted: process/src/test/java/org/apache/falcon/converter/OozieProcessMapperLateProcessTest.java
> renamed: process/src/test/java/org/apache/falcon/converter/OozieProcessMapperTest.java -> process/src/test/java/org/apache/falcon/converter/OozieProcessWorkflowBuilderTest.java
> modified: retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java
>
>
> Diffs
> -----
>
> common/src/main/java/org/apache/falcon/util/ReflectionUtils.java 4a00fa9
> common/src/main/java/org/apache/falcon/workflow/WorkflowBuilder.java 26243e7
> feed/src/main/java/org/apache/falcon/converter/OozieFeedMapper.java 2b3315f
> feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java 5e3a30e
> feed/src/test/java/org/apache/falcon/converter/OozieFeedMapperTest.java e610df2
> feed/src/test/java/org/apache/falcon/converter/OozieFeedWorkflowBuilderTest.java PRE-CREATION
> oozie/src/main/java/org/apache/falcon/converter/AbstractOozieEntityMapper.java f443939
> oozie/src/main/java/org/apache/falcon/util/OozieUtils.java 2f53370
> oozie/src/main/java/org/apache/falcon/workflow/OozieWorkflowBuilder.java e5a01ca
> oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java ac8862e
> process/src/main/java/org/apache/falcon/converter/OozieProcessMapper.java e638961
> process/src/main/java/org/apache/falcon/workflow/OozieProcessWorkflowBuilder.java 4e5e8c6
> process/src/test/java/org/apache/falcon/converter/OozieProcessMapperLateProcessTest.java fbda0ea
> process/src/test/java/org/apache/falcon/converter/OozieProcessMapperTest.java 22bf9fe
> process/src/test/java/org/apache/falcon/converter/OozieProcessWorkflowBuilderTest.java PRE-CREATION
> retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java 1e7cc04
>
> Diff: https://reviews.apache.org/r/19286/diff/
>
>
> Testing
> -------
>
> UTs
>
>
> Thanks,
>
> shwethags
>
>
Re: Review Request 19286: FALCON-356 Merge OozieProcessMapper and
OozieProcessWorkflowBuilder
Posted by sh...@inmobi.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19286/
-----------------------------------------------------------
(Updated March 18, 2014, 6:47 a.m.)
Review request for Falcon.
Changes
-------
Incorporated review comments
Repository: falcon-git
Description
-------
There is one to one mapping between OozieProcessMapper and OozieProcessWorkflowBuilder. Entity to oozie workflow mapping is spread across these two classes. Same goes for OozieFeedMapper and OozieFeedWorkflowBuilder.
modified: common/src/main/java/org/apache/falcon/util/ReflectionUtils.java
modified: common/src/main/java/org/apache/falcon/workflow/WorkflowBuilder.java
renamed: feed/src/main/java/org/apache/falcon/converter/OozieFeedMapper.java -> feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java
renamed: feed/src/test/java/org/apache/falcon/converter/OozieFeedMapperTest.java -> feed/src/test/java/org/apache/falcon/converter/OozieFeedWorkflowBuilderTest.java
modified: oozie/src/main/java/org/apache/falcon/util/OozieUtils.java
renamed: oozie/src/main/java/org/apache/falcon/converter/AbstractOozieEntityMapper.java -> oozie/src/main/java/org/apache/falcon/workflow/OozieWorkflowBuilder.java
modified: oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java
renamed: process/src/main/java/org/apache/falcon/converter/OozieProcessMapper.java -> process/src/main/java/org/apache/falcon/workflow/OozieProcessWorkflowBuilder.java
deleted: process/src/test/java/org/apache/falcon/converter/OozieProcessMapperLateProcessTest.java
renamed: process/src/test/java/org/apache/falcon/converter/OozieProcessMapperTest.java -> process/src/test/java/org/apache/falcon/converter/OozieProcessWorkflowBuilderTest.java
modified: retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java
Diffs (updated)
-----
common/src/main/java/org/apache/falcon/util/ReflectionUtils.java 4a00fa9
common/src/main/java/org/apache/falcon/workflow/WorkflowBuilder.java 26243e7
feed/src/main/java/org/apache/falcon/converter/OozieFeedMapper.java 2b3315f
feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java 5e3a30e
feed/src/test/java/org/apache/falcon/converter/OozieFeedMapperTest.java e610df2
feed/src/test/java/org/apache/falcon/converter/OozieFeedWorkflowBuilderTest.java PRE-CREATION
oozie/src/main/java/org/apache/falcon/converter/AbstractOozieEntityMapper.java f443939
oozie/src/main/java/org/apache/falcon/util/OozieUtils.java 2f53370
oozie/src/main/java/org/apache/falcon/workflow/OozieWorkflowBuilder.java e5a01ca
oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java ac8862e
process/src/main/java/org/apache/falcon/converter/OozieProcessMapper.java e638961
process/src/main/java/org/apache/falcon/workflow/OozieProcessWorkflowBuilder.java 4e5e8c6
process/src/test/java/org/apache/falcon/converter/OozieProcessMapperLateProcessTest.java fbda0ea
process/src/test/java/org/apache/falcon/converter/OozieProcessMapperTest.java 22bf9fe
process/src/test/java/org/apache/falcon/converter/OozieProcessWorkflowBuilderTest.java PRE-CREATION
retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java 1e7cc04
Diff: https://reviews.apache.org/r/19286/diff/
Testing
-------
UTs
Thanks,
shwethags
Re: Review Request 19286: FALCON-356 Merge OozieProcessMapper and
OozieProcessWorkflowBuilder
Posted by sh...@inmobi.com.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19286/
-----------------------------------------------------------
(Updated March 17, 2014, 9:50 a.m.)
Review request for Falcon.
Repository: falcon-git
Description (updated)
-------
There is one to one mapping between OozieProcessMapper and OozieProcessWorkflowBuilder. Entity to oozie workflow mapping is spread across these two classes. Same goes for OozieFeedMapper and OozieFeedWorkflowBuilder.
modified: common/src/main/java/org/apache/falcon/util/ReflectionUtils.java
modified: common/src/main/java/org/apache/falcon/workflow/WorkflowBuilder.java
renamed: feed/src/main/java/org/apache/falcon/converter/OozieFeedMapper.java -> feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java
renamed: feed/src/test/java/org/apache/falcon/converter/OozieFeedMapperTest.java -> feed/src/test/java/org/apache/falcon/converter/OozieFeedWorkflowBuilderTest.java
modified: oozie/src/main/java/org/apache/falcon/util/OozieUtils.java
renamed: oozie/src/main/java/org/apache/falcon/converter/AbstractOozieEntityMapper.java -> oozie/src/main/java/org/apache/falcon/workflow/OozieWorkflowBuilder.java
modified: oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java
renamed: process/src/main/java/org/apache/falcon/converter/OozieProcessMapper.java -> process/src/main/java/org/apache/falcon/workflow/OozieProcessWorkflowBuilder.java
deleted: process/src/test/java/org/apache/falcon/converter/OozieProcessMapperLateProcessTest.java
renamed: process/src/test/java/org/apache/falcon/converter/OozieProcessMapperTest.java -> process/src/test/java/org/apache/falcon/converter/OozieProcessWorkflowBuilderTest.java
modified: retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java
Diffs
-----
common/src/main/java/org/apache/falcon/util/ReflectionUtils.java 4a00fa9
common/src/main/java/org/apache/falcon/workflow/WorkflowBuilder.java 26243e7
feed/src/main/java/org/apache/falcon/converter/OozieFeedMapper.java 2b3315f
feed/src/main/java/org/apache/falcon/workflow/OozieFeedWorkflowBuilder.java 5e3a30e
feed/src/test/java/org/apache/falcon/converter/OozieFeedMapperTest.java e610df2
feed/src/test/java/org/apache/falcon/converter/OozieFeedWorkflowBuilderTest.java PRE-CREATION
oozie/src/main/java/org/apache/falcon/converter/AbstractOozieEntityMapper.java f443939
oozie/src/main/java/org/apache/falcon/util/OozieUtils.java 2f53370
oozie/src/main/java/org/apache/falcon/workflow/OozieWorkflowBuilder.java e5a01ca
oozie/src/main/java/org/apache/falcon/workflow/engine/OozieWorkflowEngine.java ac8862e
process/src/main/java/org/apache/falcon/converter/OozieProcessMapper.java e638961
process/src/main/java/org/apache/falcon/workflow/OozieProcessWorkflowBuilder.java 4e5e8c6
process/src/test/java/org/apache/falcon/converter/OozieProcessMapperLateProcessTest.java fbda0ea
process/src/test/java/org/apache/falcon/converter/OozieProcessMapperTest.java 22bf9fe
process/src/test/java/org/apache/falcon/converter/OozieProcessWorkflowBuilderTest.java PRE-CREATION
retention/src/test/java/org/apache/falcon/retention/FeedEvictorTest.java 1e7cc04
Diff: https://reviews.apache.org/r/19286/diff/
Testing
-------
UTs
Thanks,
shwethags