falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ajay Yadava" <ajayn...@gmail.com>
Subject Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop queue name for schedulable feed entities
Date Fri, 08 Jan 2016 04:38:34 GMT

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



common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 1225)
<https://reviews.apache.org/r/41916/#comment174024>

    I think this will be useful for processes also and it will be nice to move it outside
of FeedHelper to some place more appropriate.



common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 1260)
<https://reviews.apache.org/r/41916/#comment174022>

    This is very interesting. Can you please share the json which is treated as malformed?
I had built a custom UI for Yarn which we use at Inmobi and I just want to see how different
frameworks are handling malformed json.



common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 554)
<https://reviews.apache.org/r/41916/#comment173763>

    This won't work well in distributed mode. You are trying to validate queues for all the
clusters from each falcon server. This will result in lot of redundant validations, even if
the url were accessible from all data centers.



common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 565)
<https://reviews.apache.org/r/41916/#comment174011>

    This log statement is not required as it is getting logged below.



common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 567)
<https://reviews.apache.org/r/41916/#comment174026>

    This will validate only retention queue. Retention queue and replication queue can be
different and this will validate only retention queue.



common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 714)
<https://reviews.apache.org/r/41916/#comment174012>

    nit: Instead of string concatenation use slf4j's parameterised style which is faster and
more maintainable.



common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 715)
<https://reviews.apache.org/r/41916/#comment174010>

    nit: you can use the diamond operator to avoid IDE warnings



common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 754)
<https://reviews.apache.org/r/41916/#comment174015>

    nit: Null check is not required as IOUtils.closeQuietly handles null



common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java (line 944)
<https://reviews.apache.org/r/41916/#comment174018>

    nit: Please use diamond operator everywhere



common/src/test/resources/config/feed/feed-schedulerinfo-1.json (line 38)
<https://reviews.apache.org/r/41916/#comment174019>

    nit: please remove trailing white spaces



oozie/src/main/java/org/apache/falcon/oozie/OozieOrchestrationWorkflowBuilder.java (line 98)
<https://reviews.apache.org/r/41916/#comment174028>

    I guess you intended to use it in FeedHelper but it is not accessible there because of
module dependencies. Do we still need it as public?


- Ajay Yadava


On Jan. 5, 2016, 5:52 a.m., Venkatesan Ramachandran wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2016, 5:52 a.m.)
> 
> 
> Review request for Falcon.
> 
> 
> Repository: falcon-git
> 
> 
> Description
> -------
> 
> Validate the hadoop cluster queue name specified in the Feed entity during feed submit.
> 
> The implementation uses Resource Manager REST API to get hadoop cluster queue names.
> 
> 
> Diffs
> -----
> 
>   common/src/main/java/org/apache/falcon/entity/FeedHelper.java 150e0bd 
>   common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730

>   common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 9841083 
>   common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION 
>   common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION 
>   oozie/src/main/java/org/apache/falcon/oozie/OozieOrchestrationWorkflowBuilder.java
e137e11 
> 
> Diff: https://reviews.apache.org/r/41916/diff/
> 
> 
> Testing
> -------
> 
> Unit tests
> Manual end to end tests
> Secure cluster test
> 
> 
> Thanks,
> 
> Venkatesan Ramachandran
> 
>


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