> On Sept. 23, 2015, 10:05 p.m., Balu Vellanki wrote:
> >

Thanks Balu for reviewing and providing the comments.


> On Sept. 23, 2015, 10:05 p.m., Balu Vellanki wrote:
> > common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java,
> >  line 139
> > <https://reviews.apache.org/r/33443/diff/2/?file=1083064#file1083064line139>
> >
> >     Minor comment : 
> >      
> >      You can use the following to get counter key, values
> >      String[] keyVals = counter.split(":", 2);
> >      vertex.setProperty(keyVals[0], Long.parseLong(keyVals[1]));
> >      
> >      parseLong can throw an exception.

Done.


> On Sept. 23, 2015, 10:05 p.m., Balu Vellanki wrote:
> > oozie/src/main/java/org/apache/falcon/oozie/feed/FeedReplicationWorkflowBuilder.java,
> >  line 58
> > <https://reviews.apache.org/r/33443/diff/2/?file=1083076#file1083076line58>
> >
> >     For future, it might be useful to have FeedHelper.getPropertyValue(Feed 
> > feed, String propName), which is similar to ClusterHelper.getPropertyValue.

Thanks for bringing this up. Will take care of this in separate JIRA.


> On Sept. 23, 2015, 10:05 p.m., Balu Vellanki wrote:
> > replication/src/main/java/org/apache/falcon/replication/FeedReplicator.java,
> >  line 104
> > <https://reviews.apache.org/r/33443/diff/2/?file=1083080#file1083080line104>
> >
> >     obtainJobCounters and storeJobCounters can be combined to a single 
> > menthod, isnt it?

I could have done it in separate method as well. But here requirement is about 
obtaining counters from executed job and storing in counter file, which are two 
different functionalities. So it make more sense to have different methods for 
maintainability aspect.


- Peeyush


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


On Sept. 23, 2015, 9:02 a.m., Peeyush Bishnoi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/33443/
> -----------------------------------------------------------
> 
> (Updated Sept. 23, 2015, 9:02 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1102
>     https://issues.apache.org/jira/browse/FALCON-1102
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> FALCON-1102: Gather data transfer detail of replication job submitted from 
> HDFS recipe
> 
> 
> Diffs
> -----
> 
>   
> addons/recipes/hdfs-replication/src/main/resources/hdfs-replication-workflow.xml
>  e35773f 
>   
> common/src/main/java/org/apache/falcon/metadata/InstanceRelationshipGraphBuilder.java
>  17bf813 
>   common/src/main/java/org/apache/falcon/workflow/WorkflowExecutionArgs.java 
> 9456fb9 
>   
> common/src/main/java/org/apache/falcon/workflow/WorkflowExecutionContext.java 
> 4454239 
>   
> common/src/test/java/org/apache/falcon/metadata/MetadataMappingServiceTest.java
>  89e8178 
>   metrics/pom.xml a0358db 
>   metrics/src/main/java/org/apache/falcon/job/FSReplicationCounters.java 
> PRE-CREATION 
>   metrics/src/main/java/org/apache/falcon/job/JobCounters.java PRE-CREATION 
>   metrics/src/main/java/org/apache/falcon/job/JobCountersHandler.java 
> PRE-CREATION 
>   metrics/src/main/java/org/apache/falcon/job/JobType.java PRE-CREATION 
>   metrics/src/main/java/org/apache/falcon/job/ReplicationJobCountersList.java 
> PRE-CREATION 
>   metrics/src/test/java/org/apache/falcon/job/FSReplicationCountersTest.java 
> PRE-CREATION 
>   
> oozie/src/main/java/org/apache/falcon/oozie/feed/FSReplicationWorkflowBuilder.java
>  b82f4e0 
>   
> oozie/src/main/java/org/apache/falcon/oozie/feed/FeedReplicationWorkflowBuilder.java
>  a7c19cd 
>   
> oozie/src/test/java/org/apache/falcon/oozie/feed/OozieFeedWorkflowBuilderTest.java
>  7d0174a 
>   oozie/src/test/resources/feed/fs-replication-feed-counters.xml PRE-CREATION 
>   replication/pom.xml 3cc96fc 
>   replication/src/main/java/org/apache/falcon/replication/FeedReplicator.java 
> a226058 
> 
> Diff: https://reviews.apache.org/r/33443/diff/
> 
> 
> Testing
> -------
> 
> Yes. Unit test cases added.
> 
> 
> Thanks,
> 
> Peeyush Bishnoi
> 
>

Reply via email to