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

Reply via email to