You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Ajay Yadava <aj...@gmail.com> on 2014/09/26 22:34:58 UTC
Re: Review Request 26096: FALCON-301 Disallow feeds with same location
-----------------------------------------------------------
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
Re: Review Request 26096: FALCON-301 Disallow feeds with same location
Posted by Srikanth Sundarrajan <sr...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26096/#review55059
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95397>
Location is not present when the feed is based on catalog. Does it make sense to create a child jira to track the catalog related changes.
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95398>
If location object is present, then path, type etc can't be null. They are mandated in the schema definition
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95399>
Would it be simpler to invoke onRemove on old entity and onAdd on new entity instead ?
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95400>
This is invoked only on server start and can be treated similar to onAdd.
common/src/main/java/org/apache/falcon/entity/store/LocationStore.java
<https://reviews.apache.org/r/26096/#comment95401>
Not sure if this is the correct interface for RadixTree to implement. This seems too specific for location (at least the name). I would have considered NavigableMap or SortedMap as a viable interface for RadixTree to implement. Please evaluate the possibility and weigh the options
common/src/main/java/org/apache/falcon/util/RadixNode.java
<https://reviews.apache.org/r/26096/#comment95402>
Empty javadoc.
common/src/main/java/org/apache/falcon/util/RadixNode.java
<https://reviews.apache.org/r/26096/#comment96325>
Does it make sense for this to be package private ?
common/src/main/java/org/apache/falcon/util/RadixNode.java
<https://reviews.apache.org/r/26096/#comment96324>
Not a good idea to return the list directly, as that would allow mutation of the list outside of this class.
common/src/main/java/org/apache/falcon/util/RadixTree.java
<https://reviews.apache.org/r/26096/#comment95403>
Use @Nonnull to annotate arguments which are not null and likewise use appropriate args which are nullable.
- Srikanth Sundarrajan
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
>
>
Re: Review Request 26096: FALCON-301 Disallow feeds with same location
Posted by Suma Shivaprasad <su...@gmail.com>.
> 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
>
>
Re: Review Request 26096: FALCON-301 Disallow feeds with same location
Posted by Ajay Yadava <aj...@gmail.com>.
> 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
>
>
Re: Review Request 26096: FALCON-301 Disallow feeds with same location
Posted by Suma Shivaprasad <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26096/#review54956
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95279>
should we throw an error if EntityType != FEED is passed?
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95281>
could use StringUtils from apache-commons
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95282>
same as above. could use StringUtils.trim..which removes ctrl chars and whitespace
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95280>
same as above
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95283>
will one of the delete fails cause the entire feed to be aborted? Instead could we catch the Exception and log the error cases
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95285>
same as above
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95284>
same as above
common/src/main/java/org/apache/falcon/util/RadixTree.java
<https://reviews.apache.org/r/26096/#comment95286>
could remove extra blank lines
- Suma Shivaprasad
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
>
>
Re: Review Request 26096: FALCON-301 Disallow feeds with same location
Posted by Ajay Yadava <aj...@gmail.com>.
> On Sept. 30, 2014, 12:59 p.m., Suma Shivaprasad wrote:
> > common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java, line 42
> > <https://reviews.apache.org/r/26096/diff/1/?file=706326#file706326line42>
> >
> > Can this state be reconstructed on restart through listener or do we need any serialiation/deserialization for the state?
Yes it can be reconstructed through listener.
- Ajay
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26096/#review54971
-----------------------------------------------------------
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
>
>
Re: Review Request 26096: FALCON-301 Disallow feeds with same location
Posted by Suma Shivaprasad <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26096/#review54971
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95302>
Can this state be reconstructed on restart through listener or do we need any serialiation/deserialization for the state?
- Suma Shivaprasad
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
>
>
Re: Review Request 26096: FALCON-301 Disallow feeds with same location
Posted by Suma Shivaprasad <su...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26096/#review54957
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95287>
Also delete check for boolean is missing in all places
common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java
<https://reviews.apache.org/r/26096/#comment95288>
delete result (boolean) check is missing in all calls
- Suma Shivaprasad
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
>
>