falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ying Zheng" <yzh...@hortonworks.com>
Subject Re: Review Request 32946: Falcon-1121: Backend Support for Free-text Entity Search
Date Fri, 17 Apr 2015 01:12:27 GMT


> On April 8, 2015, 8:59 p.m., Scott Preece wrote:
> > client/src/main/java/org/apache/falcon/cli/FalconCLI.java, line 100
> > <https://reviews.apache.org/r/32946/diff/1/?file=920312#file920312line100>
> >
> >     "NAMESEQ" doesn't suggest anything to me - "NAME_PATTERN" would be clearer.

This is to match the api parameter "nameseq". "seq" is the abbreviation for "sequence", since
we provide subsequence matching for entity name. We would like to be more specific about what
kind of pattern we are handling with.


> On April 8, 2015, 8:59 p.m., Scott Preece wrote:
> > client/src/main/java/org/apache/falcon/resource/EntityList.java, line 76
> > <https://reviews.apache.org/r/32946/diff/1/?file=920315#file920315line76>
> >
> >     If this is a list, should it be "clusters"?

The parent xml node is named as "clusters" (see line 75). This is to be consistent with the
current output style (see line 71-74 for tags and pipelines).


> On April 8, 2015, 8:59 p.m., Scott Preece wrote:
> > client/src/main/java/org/apache/falcon/resource/EntityList.java, line 132
> > <https://reviews.apache.org/r/32946/diff/1/?file=920315#file920315line132>
> >
> >     As above, if it's a list, shouldn't it be "clusters"?

This is the same question as your previous comment. See reply above.


> On April 8, 2015, 8:59 p.m., Scott Preece wrote:
> > common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java, line
79
> > <https://reviews.apache.org/r/32946/diff/1/?file=920316#file920316line79>
> >
> >     Make this
> >     public String getTags() {
> >         return null;
> >     }
> >     
> >     And should it return "NULL" as in line 75 or an actual null?

In Java, null is a built-in literal (just as true, false). It is used to identify a variable
as not referencing any object. Maybe you are thinking about NULL in c++, where NULL is a macro?
Java has no concept of macro and there is no compelling reason to make it uppercase.


> On April 8, 2015, 8:59 p.m., Scott Preece wrote:
> > docs/src/site/twiki/restapi/EntityList.twiki, line 11
> > <https://reviews.apache.org/r/32946/diff/1/?file=920318#file920318line11>
> >
> >     Make "cluster, feed or process or schedulable" become "cluster, feed, process,
or schedulable"

Good catch! Removed additional "or".


> On April 8, 2015, 8:59 p.m., Scott Preece wrote:
> > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java, lines
570-573
> > <https://reviews.apache.org/r/32946/diff/1/?file=920319#file920319line570>
> >
> >     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"

Thanks for suggestions, Scott. I changed to "Tag keywords to match, seperated by commma".


> On April 8, 2015, 8:59 p.m., Scott Preece wrote:
> > prism/src/main/java/org/apache/falcon/resource/AbstractEntityManager.java, line
585
> > <https://reviews.apache.org/r/32946/diff/1/?file=920319#file920319line585>
> >
> >     Why the change to upper case?

Because when we compare 'fields' with EntityFieldList to see which fields are needed in the
output (see getEntityElement, line 961-977) and these enum names are in upper case.


- Ying


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32946/#review79411
-----------------------------------------------------------


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


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