> On Sept. 30, 2014, 7:06 a.m., Suma Shivaprasad wrote:
> > common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java, 
> > line 90
> > <https://reviews.apache.org/r/26096/diff/1/?file=706326#file706326line90>
> >
> >     will one of the delete fails cause the entire feed to be aborted? 
> > Instead could we catch the Exception and log the error cases
> 
> Ajay Yadava wrote:
>     I think catching exception and suppressing will be wrong here. It will 
> fail in cases like where you delete a feed and one of locations failed to 
> delete and then you try to resubmit it, falcon will not let you submit it and 
> there is no other way to delete it.

yeah makes sense


> On Sept. 30, 2014, 7:06 a.m., Suma Shivaprasad wrote:
> > common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java, 
> > line 64
> > <https://reviews.apache.org/r/26096/diff/1/?file=706326#file706326line64>
> >
> >     could use StringUtils from apache-commons
> 
> Ajay Yadava wrote:
>     Does it provide more funcitonality than this for isEmpty()?

StringUtils.isBlank - Checks if a String is whitespace, empty ("") or null 
whereas empty checks for and is more concise


- Suma


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


On Sept. 26, 2014, 8:34 p.m., Ajay Yadava wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26096/
> -----------------------------------------------------------
> 
> (Updated Sept. 26, 2014, 8:34 p.m.)
> 
> 
> Review request for Falcon and Srikanth Sundarrajan.
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> FALCON-301 Disallow feeds with same location
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java 
> PRE-CREATION 
>   common/src/main/java/org/apache/falcon/entity/store/LocationStore.java 
> PRE-CREATION 
>   common/src/main/java/org/apache/falcon/util/KeyAlreadyExistsException.java 
> PRE-CREATION 
>   common/src/main/java/org/apache/falcon/util/RadixNode.java PRE-CREATION 
>   common/src/main/java/org/apache/falcon/util/RadixTree.java PRE-CREATION 
>   common/src/main/resources/startup.properties e233b2a 
>   common/src/test/java/org/apache/falcon/entity/AbstractTestBase.java 2140335 
>   
> common/src/test/java/org/apache/falcon/entity/store/FeedLocationStoreTest.java
>  PRE-CREATION 
>   common/src/test/java/org/apache/falcon/group/FeedGroupMapTest.java a6c52e3 
>   common/src/test/java/org/apache/falcon/util/RadixNodeTest.java PRE-CREATION 
>   common/src/test/java/org/apache/falcon/util/RadixTreeTest.java PRE-CREATION 
>   src/conf/startup.properties 78466af 
>   webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java 0943103 
>   webapp/src/test/java/org/apache/falcon/cli/FalconCLISmokeIT.java cb0dd2d 
>   webapp/src/test/java/org/apache/falcon/process/PigProcessIT.java 0f2a971 
>   webapp/src/test/java/org/apache/falcon/process/TableStorageProcessIT.java 
> 51afbb8 
>   webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java 
> ed70a0b 
>   
> webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseySmokeIT.java
>  d4a1d8a 
>   
> webapp/src/test/java/org/apache/falcon/resource/EntityManagerPaginationJerseyIT.java
>  bd68e57 
>   
> webapp/src/test/java/org/apache/falcon/resource/MetadataResourceJerseyIT.java 
> 5249888 
>   webapp/src/test/java/org/apache/falcon/resource/TestContext.java e9545d1 
>   webapp/src/test/resources/feed-template1.xml 456f7ce 
>   webapp/src/test/resources/feed-template2.xml d4901fa 
> 
> Diff: https://reviews.apache.org/r/26096/diff/
> 
> 
> Testing
> -------
> 
> Yes. Unit Tests are present for new classes and all unit & integration tests 
> pass.
> 
> 
> Thanks,
> 
> Ajay Yadava
> 
>

Reply via email to