falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From shwetha...@inmobi.com
Subject Re: Review Request 19286: FALCON-356 Merge OozieProcessMapper and OozieProcessWorkflowBuilder
Date Tue, 18 Mar 2014 06:02:21 GMT


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


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