> On Sept. 30, 2014, 7:06 a.m., Suma Shivaprasad wrote: > > common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java, > > line 59 > > <https://reviews.apache.org/r/26096/diff/1/?file=706326#file706326line59> > > > > should we throw an error if EntityType != FEED is passed?
IMHO we shouldn't, other listeners might be interested in this event. > 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 Does it provide more funcitonality than this for isEmpty()? > On Sept. 30, 2014, 7:06 a.m., Suma Shivaprasad wrote: > > common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java, > > line 66 > > <https://reviews.apache.org/r/26096/diff/1/?file=706326#file706326line66> > > > > same as above. could use StringUtils.trim..which removes ctrl chars and > > whitespace Will switch for this case. > 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 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. > On Sept. 30, 2014, 7:06 a.m., Suma Shivaprasad wrote: > > common/src/main/java/org/apache/falcon/util/RadixTree.java, line 167 > > <https://reviews.apache.org/r/26096/diff/1/?file=706330#file706330line167> > > > > could remove extra blank lines Will do. - Ajay ----------------------------------------------------------- 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 > >
