falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sowmya Ramesh" <sram...@hortonworks.com>
Subject Re: Review Request 38391: Simple validation for cluster entity properties
Date Wed, 16 Sep 2015 06:46:20 GMT

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



common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java (line 345)
<https://reviews.apache.org/r/38391/#comment156096>

    Can you use isBlank instead?



common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java (line 348)
<https://reviews.apache.org/r/38391/#comment156097>

    Minor nit: Hashset add returns false if element is already present



common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java (line 293)
<https://reviews.apache.org/r/38391/#comment156098>

    I see same logic in mutiple places. Can we add a generic api say validateProperties in
Entity util and reuse it?


- Sowmya Ramesh


On Sept. 15, 2015, 4:04 p.m., Balu Vellanki wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38391/
> -----------------------------------------------------------
> 
> (Updated Sept. 15, 2015, 4:04 p.m.)
> 
> 
> Review request for Falcon, Pallavi Rao, Sowmya Ramesh, and Venkat Ranganathan.
> 
> 
> Bugs: FALCON-1342
>     https://issues.apache.org/jira/browse/FALCON-1342
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> When specifying properties for a cluster, this is currently allowed,
> 
> {code}
> <properties>
>         <property name="test" value="value1"/>
>         <property name="test" value="value2"/>
> </properties>
> {code}
> 
> The propeties are stored as an array of org.apache.falcon.entity.v0.cluster.Property,
and cluster.getProperty("test") will return either "value1" or "value2" but not both. If falcon
does not support multiple values for same property key,  parsing such an entity should throw
an error.
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java 5756f84

>   common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java f22a343

>   common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 56cc4ca

>   common/src/test/java/org/apache/falcon/entity/parser/ClusterEntityParserTest.java 638cef9

>   common/src/test/java/org/apache/falcon/entity/parser/FeedEntityParserTest.java 754fb06

>   common/src/test/java/org/apache/falcon/entity/parser/ProcessEntityParserTest.java 6612b74

>   common/src/test/resources/config/feed/feed-0.1.xml 6448803 
> 
> Diff: https://reviews.apache.org/r/38391/diff/
> 
> 
> Testing
> -------
> 
> Added unit test. Tested end2end
> 
> 
> Thanks,
> 
> Balu Vellanki
> 
>


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