falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Srikanth Sundarrajan" <srik...@hotmail.com>
Subject Re: Review Request 31660: Falcon-822: Exposed API for reverse lookup
Date Wed, 04 Mar 2015 03:34:08 GMT

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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message