----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31660/#review75146 -----------------------------------------------------------
client/src/main/java/org/apache/falcon/cli/FalconCLI.java <https://reviews.apache.org/r/31660/#comment122091> One thing for us to think about is if this is applicable only for file system based feeds. -path switch seems to suggest that it is restricted to FS based feed. Also we need to see how the radix tree would hold catalog based instance. Catalog related work can happen in an independent jira, but it is important to establish this correctly as an external facing contract. common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java <https://reviews.apache.org/r/31660/#comment122094> Does it make sense for FeedHelper.getLocations to get clusterSpecificLocation always. This seems to be repeated in many places (outside of this patch) and might be good to cleanup a bit. common/src/main/java/org/apache/falcon/util/RadixTree.java <https://reviews.apache.org/r/31660/#comment122095> You need this at debug level or trace ? common/src/main/java/org/apache/falcon/util/RadixTree.java <https://reviews.apache.org/r/31660/#comment122096> Too much logging ? common/src/main/resources/log4j.xml <https://reviews.apache.org/r/31660/#comment122097> Why is this change needed ? Is it accidentally left over post a debug session ? docs/src/site/twiki/FalconCLI.twiki <https://reviews.apache.org/r/31660/#comment122098> Few examples to cover some scenarios might help the users a lot. prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java <https://reviews.apache.org/r/31660/#comment122099> Should perhaps be "Reverse lookup not supported for this entityType" prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java <https://reviews.apache.org/r/31660/#comment122100> Not clear what is the significance of the "/" at the end. There are two calls made, one with slash and one without. Can you elaborate and include this as code comments - Srikanth Sundarrajan On March 3, 2015, 3:30 a.m., Ajay Yadava wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31660/ > ----------------------------------------------------------- > > (Updated March 3, 2015, 3:30 a.m.) > > > Review request for Falcon and Srikanth Sundarrajan. > > > Bugs: https://issues.apache.org/jira/browse/FALCON-822 > > https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/FALCON-822 > > > Repository: falcon-git > > > Description > ------- > > Falcon-822: Exposed an API over radix tree to enable users to reverse look up > the feed name using a feed instance path. Also addressed FALCON-1037 to > handle trailing slashes. First set of review comments given in JIRA are also > addressed > > > Diffs > ----- > > client/src/main/java/org/apache/falcon/ResponseHelper.java 5f36e3a > client/src/main/java/org/apache/falcon/cli/FalconCLI.java ac76a9c > client/src/main/java/org/apache/falcon/client/FalconClient.java 86397c4 > client/src/main/java/org/apache/falcon/resource/FeedLookupResult.java > PRE-CREATION > common/src/main/java/org/apache/falcon/entity/store/FeedLocationStore.java > e056d96 > common/src/main/java/org/apache/falcon/util/FalconRadixUtils.java bbd73c7 > common/src/main/java/org/apache/falcon/util/RadixTree.java 6cd79f5 > common/src/main/resources/log4j.xml 75c8267 > common/src/main/resources/startup.properties 433c2a8 > > common/src/test/java/org/apache/falcon/entity/store/FeedLocationStoreTest.java > 86ef775 > common/src/test/java/org/apache/falcon/util/RadixTreeTest.java 109c24d > docs/src/site/twiki/FalconCLI.twiki 547aa7d > docs/src/site/twiki/restapi/FeedLookup.twiki PRE-CREATION > docs/src/site/twiki/restapi/ResourceList.twiki 2f37bb3 > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java > 424bada > > prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java > 5f711ee > prism/src/test/java/org/apache/falcon/resource/EntityManagerTest.java > 470f866 > src/conf/startup.properties 2db4b1e > > webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java > a83f0cf > webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java d46f112 > > Diff: https://reviews.apache.org/r/31660/diff/ > > > Testing > ------- > > Unit tests and Integration tests are added. > > ( I have also addresssed the first set of code review comments & Falcon-1037 > as well to handle trailing slashes) > > > Thanks, > > Ajay Yadava > >
