----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32946/#review79411 -----------------------------------------------------------
All my comments are style things - I didn't notice anything that would keep it from working. client/src/main/java/org/apache/falcon/cli/FalconCLI.java <https://reviews.apache.org/r/32946/#comment128739> "NAMESEQ" doesn't suggest anything to me - "NAME_PATTERN" would be clearer. client/src/main/java/org/apache/falcon/cli/FalconCLI.java <https://reviews.apache.org/r/32946/#comment128783> If tagkey is actually a list of tag keywords, then "tagkeys" would be clearer. client/src/main/java/org/apache/falcon/resource/EntityList.java <https://reviews.apache.org/r/32946/#comment128784> If this is a list, should it be "clusters"? client/src/main/java/org/apache/falcon/resource/EntityList.java <https://reviews.apache.org/r/32946/#comment128785> As above, if it's a list, shouldn't it be "clusters"? common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java <https://reviews.apache.org/r/32946/#comment128758> Make this public String getTags() { return null; } And should it return "NULL" as in line 75 or an actual null? docs/src/site/twiki/restapi/EntityList.twiki <https://reviews.apache.org/r/32946/#comment128759> Make "cluster, feed or process or schedulable" become "cluster, feed, process, or schedulable" prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java <https://reviews.apache.org/r/32946/#comment128777> Probably should state exactly what "filter by" means for each filter type - for example, * @param tagKey Filter by tag keywords, separated by comma; keep entity only if it has all given tags" prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java <https://reviews.apache.org/r/32946/#comment128764> Why the change to upper case? prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java <https://reviews.apache.org/r/32946/#comment128770> Personally, I would find these method names clearer if they were like "matchesNameSubsequence" and they returned true when the entity matches. "filteredBy" is a very clear concept, IFF you already know what it means. Maybe it would be clearer if the names were like "is ExcludedBy", but basically it's confusing to return true to mean "doesn't pass the filter". - Scott Preece On April 8, 2015, 1:05 a.m., Ying Zheng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/32946/ > ----------------------------------------------------------- > > (Updated April 8, 2015, 1:05 a.m.) > > > Review request for Falcon, Ajay Yadava, Sowmya Ramesh, Seetharam Venkatesh, > and Venkat Ranganathan. > > > Bugs: Falcon-1121 > https://issues.apache.org/jira/browse/Falcon-1121 > > > Repository: falcon-git > > > Description > ------- > > See parent JIRA Falcon-1095 for detailed descriptions: > http://issues.apache.org/jira/browse/Falcon-1095 > Four changes in total: > 1. Added API support: return feeds and processes if :entity-type is set to > schedulable; change parameter for subsequence matching from "pattern" to > "nameseq"; multi-keyword matching for entity tags with parameter "tagkey". > 2. Added command line support. > 3. Added unit tests. > 4. Updated twiki. > > > Diffs > ----- > > client/src/main/java/org/apache/falcon/cli/FalconCLI.java 958ea62 > client/src/main/java/org/apache/falcon/client/FalconClient.java 15691a6 > client/src/main/java/org/apache/falcon/entity/v0/Entity.java 7fb271d > client/src/main/java/org/apache/falcon/resource/EntityList.java d43418c > common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java > 1c1c325 > docs/src/site/twiki/FalconCLI.twiki 22ffbe7 > docs/src/site/twiki/restapi/EntityList.twiki 5e11691 > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java > c0df4d6 > > prism/src/main/java/org/apache/falcon/resource/AbstractSchedulableEntityManager.java > f7c2f61 > > prism/src/main/java/org/apache/falcon/resource/proxy/SchedulableEntityManagerProxy.java > 8bfc099 > prism/src/test/java/org/apache/falcon/resource/EntityManagerTest.java > 6f75111 > > webapp/src/main/java/org/apache/falcon/resource/SchedulableEntityManager.java > 261a9a9 > webapp/src/test/java/org/apache/falcon/cli/FalconCLIIT.java bfad011 > webapp/src/test/java/org/apache/falcon/cli/FalconCLISmokeIT.java ee5534a > webapp/src/test/java/org/apache/falcon/resource/EntityManagerJerseyIT.java > 280253d > > Diff: https://reviews.apache.org/r/32946/diff/ > > > Testing > ------- > > Unit tests passed. Also tested command line and using api calls. > > > Thanks, > > Ying Zheng > >
