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