falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Peeyush Bishnoi" <bpeey...@yahoo.co.in>
Subject Re: Review Request 41601: Add CLI option to display captured replication metrics
Date Fri, 08 Jan 2016 14:42:26 GMT


> On Jan. 7, 2016, 9:20 p.m., Balu Vellanki wrote:
> >

@Balu, Thanks a lot for reviewing this.


> On Jan. 7, 2016, 9:20 p.m., Balu Vellanki wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconMetadataCLI.java, line 162
> > <https://reviews.apache.org/r/41601/diff/6/?file=1186373#file1186373line162>
> >
> >     This line is repeated and can be outside if-else block

Fixed.


> On Jan. 7, 2016, 9:20 p.m., Balu Vellanki wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconMetadataCLI.java, line 249
> > <https://reviews.apache.org/r/41601/diff/6/?file=1186373#file1186373line249>
> >
> >     Please use StringUtils.isNotBlank()

I could have used the StringUtils.isBlank but I have tried to align with other implementation
as done earlier for validation.
Fixed.


> On Jan. 7, 2016, 9:20 p.m., Balu Vellanki wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconMetadataCLI.java, line 253
> > <https://reviews.apache.org/r/41601/diff/6/?file=1186373#file1186373line253>
> >
> >     It is better to use StringUtils.isNotBlank() method instead.

I could have used the StringUtils.isBlank but I have tried to align with other implementation
as done earlier for validation.
Fixed.


> On Jan. 7, 2016, 9:20 p.m., Balu Vellanki wrote:
> > client/src/main/java/org/apache/falcon/client/FalconClient.java, line 242
> > <https://reviews.apache.org/r/41601/diff/6/?file=1186374#file1186374line242>
> >
> >     If you are listing replication metrics, then this operation is not necessary.
You should handle it as part of the LIST operation.

fixed.


> On Jan. 7, 2016, 9:20 p.m., Balu Vellanki wrote:
> > prism/src/main/java/org/apache/falcon/resource/metadata/MetadataDiscoveryResource.java,
line 108
> > <https://reviews.apache.org/r/41601/diff/6/?file=1186377#file1186377line108>
> >
> >     replication-metrics is the {type} in listDimensionValues method above, it is
not needed to write a separate method here.

If you have followed up the discussion with Ajay, earlier I have done impementation to handle
replication-metrics in single method only. But for that single method there will be a long
list of parameters, many of which is not required by other API's. And replication-metrics
is not for all the entities but specifically to list the replication metrics from the repliction
recipe's and feed replication. That's why I have used the   @Path("replication-metrics/list")
.  

As I have mentioned in my earlier comment that we have to think to refactor sendMetadataDiscoveryRequest
method to separate Metadiscovery List and Relations and then we can accomodate replication-metrics
in Metadiscovery List appropriately. I will create the JIRA issue for refactoring the sendMetadataDiscoveryRequest
method.


- Peeyush


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


On Jan. 7, 2016, 6:23 p.m., Peeyush Bishnoi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41601/
> -----------------------------------------------------------
> 
> (Updated Jan. 7, 2016, 6:23 p.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Bugs: FALCON-1643
>     https://issues.apache.org/jira/browse/FALCON-1643
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> FALCON-1643 :  Add CLI option to display captured replication metrics
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/falcon/cli/FalconCLI.java 24f230a 
>   client/src/main/java/org/apache/falcon/cli/FalconMetadataCLI.java 36dd613 
>   client/src/main/java/org/apache/falcon/client/FalconClient.java aea39a6 
>   client/src/main/java/org/apache/falcon/metadata/RelationshipType.java 8e5f8ea 
>   docs/src/site/twiki/FalconCLI.twiki 26e6b33 
>   prism/src/main/java/org/apache/falcon/resource/metadata/MetadataDiscoveryResource.java
60c1089 
>   prism/src/test/java/org/apache/falcon/resource/metadata/MetadataDiscoveryResourceTest.java
84ada9a 
>   prism/src/test/java/org/apache/falcon/resource/metadata/MetadataTestContext.java 05cc2e9

> 
> Diff: https://reviews.apache.org/r/41601/diff/
> 
> 
> Testing
> -------
> 
> Unit test cased added. Yes.
> 
> 
> Thanks,
> 
> Peeyush Bishnoi
> 
>


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