You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@falcon.apache.org by Venkatesan Ramachandran <me...@gmail.com> on 2016/01/05 06:52:27 UTC
Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/
-----------------------------------------------------------
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
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Venkatesan Ramachandran <me...@gmail.com>.
> On Jan. 5, 2016, 9:50 p.m., Balu Vellanki wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 1183
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181570#file1181570line1183>
> >
> > What is the purpose of this line?
It's getting the retentionLimit and converting to seconds as per the function.
retentionLimit = getCluster(feed, clusterName).getRetention().getLimit();
> On Jan. 5, 2016, 9:50 p.m., Balu Vellanki wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 1205
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181570#file1181570line1205>
> >
> > Minor nit : Instead of old (which is a loaded word), can we call this getRetentionQueueFromProperties ?
This is from previous patch. But I have refactored it to getRetentionQueueFromProperties()
> On Jan. 5, 2016, 9:50 p.m., Balu Vellanki wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 1216
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181570#file1181570line1216>
> >
> > A reco for future : It would help to have a getProperty(Entity, propertyKey) method in EntityUtil instead of duplicating same code in ClusterHelper and FeedHelper.
should be addressed in a refactor patch.
- Venkatesan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review112934
-----------------------------------------------------------
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
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review112934
-----------------------------------------------------------
1. If you are doing this for feed, you should do the same for process entity as well.
2. Please add documentation that tells users that queue validation is done only if the property "yarn.resourcemanager.webapp.https.address" exists in cluster entity.
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 1183)
<https://reviews.apache.org/r/41916/#comment173423>
What is the purpose of this line?
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 1205)
<https://reviews.apache.org/r/41916/#comment173424>
Minor nit : Instead of old (which is a loaded word), can we call this getRetentionQueueFromProperties ?
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 1216)
<https://reviews.apache.org/r/41916/#comment173426>
A reco for future : It would help to have a getProperty(Entity, propertyKey) method in EntityUtil instead of duplicating same code in ClusterHelper and FeedHelper.
- Balu Vellanki
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
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Venkatesan Ramachandran <me...@gmail.com>.
> On Jan. 8, 2016, 4:38 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 1225
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181570#file1181570line1225>
> >
> > 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.
Moved to an Utility class.
> On Jan. 8, 2016, 4:38 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 1260
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181570#file1181570line1260>
> >
> > 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.
examine the returned json "health" node. There are repeating "entry" nodes but inside an array "[".
Old json parsers ignore it but if you use the latest ones, you'll get an exception.
> On Jan. 8, 2016, 4:38 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, line 554
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181571#file1181571line554>
> >
> > 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.
Not sure what you mean. Can you add more details? I also pinged you on this, but yet to hear back.
> On Jan. 8, 2016, 4:38 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, line 567
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181571#file1181571line567>
> >
> > This will validate only retention queue. Retention queue and replication queue can be different and this will validate only retention queue.
Currently, Lifecycle does not support Replication and so the replication Q is specified via "queueName" property and it is handled.
> On Jan. 8, 2016, 4:38 a.m., Ajay Yadava wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, line 715
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181571#file1181571line715>
> >
> > nit: you can use the diamond operator to avoid IDE warnings
We have made Java7 as the baseline and so it's okay.
- Venkatesan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review112898
-----------------------------------------------------------
On Feb. 10, 2016, 10:32 p.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 10:32 p.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 b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Ajay Yadava <aj...@gmail.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Venkatesan Ramachandran <me...@gmail.com>.
> On Jan. 7, 2016, 10:49 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 1217
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181570#file1181570line1217>
> >
> > can feed.getProperties() return nul?
Looks like JAXP initialized to empty list. but anyways I added a null check to be on the safer side.
> On Jan. 7, 2016, 10:49 p.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, line 555
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181571#file1181571line555>
> >
> > Which is the clustertype check required? What happens in case of replication feed?
Now checks both source and target clusters.
- Venkatesan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review113348
-----------------------------------------------------------
On Feb. 10, 2016, 10:32 p.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 10:32 p.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 b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review113348
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 1217)
<https://reviews.apache.org/r/41916/#comment173884>
can feed.getProperties() return nul?
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 1228)
<https://reviews.apache.org/r/41916/#comment173886>
nit: Please use LOG.debug("{}", ) format instead of +
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 555)
<https://reviews.apache.org/r/41916/#comment173889>
Which is the clustertype check required? What happens in case of replication feed?
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 572)
<https://reviews.apache.org/r/41916/#comment173890>
nit: use {} instead of + for logging
- Sowmya Ramesh
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
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Venkatesan Ramachandran <me...@gmail.com>.
> On Jan. 27, 2016, 3:10 a.m., sandeep samudrala wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, line 555
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181571#file1181571line555>
> >
> > In case of distributed mode, if this validation is not happening for each cluster(at serverlevel) defined in the feed, this may not work.
Checking all the clusters.
> On Jan. 27, 2016, 3:10 a.m., sandeep samudrala wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, line 567
> > <https://reviews.apache.org/r/41916/diff/1/?file=1181571#file1181571line567>
> >
> > getRetentionQueue will validate only the queue mentioned in lifecycle if present and not the normal retention queue mentioned in the feed definition.
First, tries Lifecycle and if that is null, gets the queue name from the feed property.
public static String getRetentionQueue(Feed feed, Cluster feedCluster) throws FalconException {
String queueName;
queueName = getLifecycleRetentionQueue(feed, feedCluster.getName());
if (queueName == null) {
queueName = getQueueFromProperties(feed);
}
return queueName;
}
- Venkatesan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review116527
-----------------------------------------------------------
On Feb. 10, 2016, 10:32 p.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 10:32 p.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 b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by sandeep samudrala <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review116527
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 555)
<https://reviews.apache.org/r/41916/#comment177539>
In case of distributed mode, if this validation is not happening for each cluster(at serverlevel) defined in the feed, this may not work.
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 567)
<https://reviews.apache.org/r/41916/#comment177540>
getRetentionQueue will validate only the queue mentioned in lifecycle if present and not the normal retention queue mentioned in the feed definition.
- sandeep samudrala
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
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Venkatesan Ramachandran <me...@gmail.com>.
> On Feb. 10, 2016, 11:13 p.m., Balu Vellanki wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java, line 330
> > <https://reviews.apache.org/r/41916/diff/2/?file=1239855#file1239855line330>
> >
> > Nit : Please use string constant for "queueName" as it is used in multiple locations. One suggested location is EntityUtil.java
There is a MR_QUEUE_NAME in one of the oozie utils class, but that project is not linked with commons. Since this is being used now in multiple places, I have added another constant.
> On Feb. 10, 2016, 11:13 p.m., Balu Vellanki wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, line 600
> > <https://reviews.apache.org/r/41916/diff/1-2/?file=1181571#file1181571line600>
> >
> > Minor Nit : This check is not necessary for Set and HashSet
Yup, I was initially using an ArrayList and then switched to Set and forgot about it.
> On Feb. 10, 2016, 11:13 p.m., Balu Vellanki wrote:
> > common/src/main/java/org/apache/falcon/entity/FeedHelper.java, line 1273
> > <https://reviews.apache.org/r/41916/diff/2/?file=1239853#file1239853line1273>
> >
> > Nit : Look in ClusterHelper.java at method getPropertyValue(Cluster cluster, String propName). It is much cleaner to implement a similar method in FeedHelper and use that.
> >
> > Also, please use a String constant for "queueName" as it is used in multiple locations.
getQueueFromProperties () does the same thing. Just made it to accept a parameter so that it can be generic.
- Venkatesan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review118751
-----------------------------------------------------------
On Feb. 10, 2016, 10:32 p.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 10:32 p.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 b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review118751
-----------------------------------------------------------
Ship it!
Fix it and ship it. Code looks good.
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 588)
<https://reviews.apache.org/r/41916/#comment180050>
Minor Nit : This check is not necessary for Set and HashSet
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 1273)
<https://reviews.apache.org/r/41916/#comment180052>
Nit : Look in ClusterHelper.java at method getPropertyValue(Cluster cluster, String propName). It is much cleaner to implement a similar method in FeedHelper and use that.
Also, please use a String constant for "queueName" as it is used in multiple locations.
common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java (line 330)
<https://reviews.apache.org/r/41916/#comment180056>
Nit : Please use string constant for "queueName" as it is used in multiple locations. One suggested location is EntityUtil.java
- Balu Vellanki
On Feb. 10, 2016, 10:32 p.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 10:32 p.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 b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review119526
-----------------------------------------------------------
Ship it!
Ship It!
- Sowmya Ramesh
On Feb. 11, 2016, 11:53 p.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 11:53 p.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/EntityUtil.java 24dbf3d
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by sandeep samudrala <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review119693
-----------------------------------------------------------
Ship it!
Ship It!
- sandeep samudrala
On Feb. 11, 2016, 11:53 p.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 11:53 p.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/EntityUtil.java 24dbf3d
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Balu Vellanki <bv...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review119686
-----------------------------------------------------------
Ship it!
+1 will commit shortly
- Balu Vellanki
On Feb. 11, 2016, 11:53 p.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 11:53 p.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/EntityUtil.java 24dbf3d
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Venkatesan Ramachandran <me...@gmail.com>.
> On Feb. 18, 2016, 7:16 a.m., sandeep samudrala wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, line 579
> > <https://reviews.apache.org/r/41916/diff/4/?file=1240484#file1240484line579>
> >
> > In case of a feed which has no replication and lifecycle retention is defined.
> > Then it would validate only the queue in the lifecycle retention and not the one mentioned in the properties in the feed definition.
> >
> > Should that be fine. I feel, both the queues mentioned in the feed definition needs to be validate. Thoughts please
That should be fine.
Validates in the following order:
1. if retention is enabled,
1a. check for the queue name specified in the lifecycle
1b. if not, use the queue name from the property
2. if replication is enabled,
2a. check the queue name from property
- Venkatesan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review119594
-----------------------------------------------------------
On Feb. 11, 2016, 11:53 p.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 11:53 p.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/EntityUtil.java 24dbf3d
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by sandeep samudrala <sa...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review119594
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 579)
<https://reviews.apache.org/r/41916/#comment180952>
In case of a feed which has no replication and lifecycle retention is defined.
Then it would validate only the queue in the lifecycle retention and not the one mentioned in the properties in the feed definition.
Should that be fine. I feel, both the queues mentioned in the feed definition needs to be validate. Thoughts please
- sandeep samudrala
On Feb. 11, 2016, 11:53 p.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 11:53 p.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/EntityUtil.java 24dbf3d
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Venkatesan Ramachandran <me...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/
-----------------------------------------------------------
(Updated Feb. 11, 2016, 11:53 p.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 (updated)
-----
common/src/main/java/org/apache/falcon/entity/EntityUtil.java 24dbf3d
common/src/main/java/org/apache/falcon/entity/FeedHelper.java b3aaaab
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
Diff: https://reviews.apache.org/r/41916/diff/
Testing
-------
Unit tests
Manual end to end tests
Secure cluster test
Thanks,
Venkatesan Ramachandran
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Venkatesan Ramachandran <me...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/
-----------------------------------------------------------
(Updated Feb. 11, 2016, 12:26 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 (updated)
-----
common/src/main/java/org/apache/falcon/entity/EntityUtil.java 24dbf3d
common/src/main/java/org/apache/falcon/entity/FeedHelper.java b3aaaab
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
Diff: https://reviews.apache.org/r/41916/diff/
Testing
-------
Unit tests
Manual end to end tests
Secure cluster test
Thanks,
Venkatesan Ramachandran
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Venkatesan Ramachandran <me...@gmail.com>.
> On Feb. 11, 2016, 12:25 a.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, line 554
> > <https://reviews.apache.org/r/41916/diff/2/?file=1239854#file1239854line554>
> >
> > nit: Use constants
used only one place.
> On Feb. 11, 2016, 12:25 a.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, lines 567-569
> > <https://reviews.apache.org/r/41916/diff/2/?file=1239854#file1239854line567>
> >
> > can we use LOG.info(" {}", ) instead of format
the same message is logged and throws in exception.
> On Feb. 11, 2016, 12:25 a.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java, line 588
> > <https://reviews.apache.org/r/41916/diff/2/?file=1239854#file1239854line588>
> >
> > Since queueList is set, contains check not required.
N/A
> On Feb. 11, 2016, 12:25 a.m., Sowmya Ramesh wrote:
> > common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java, line 355
> > <https://reviews.apache.org/r/41916/diff/2/?file=1239855#file1239855line355>
> >
> > use LOG.info({}, ) instead of string format
message is logged and thown.
- Venkatesan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review118761
-----------------------------------------------------------
On Feb. 11, 2016, 12:26 a.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 11, 2016, 12:26 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/EntityUtil.java 24dbf3d
> common/src/main/java/org/apache/falcon/entity/FeedHelper.java b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Sowmya Ramesh <sr...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/#review118761
-----------------------------------------------------------
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 1223)
<https://reviews.apache.org/r/41916/#comment180064>
same line repeated again, typo?
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 1242)
<https://reviews.apache.org/r/41916/#comment180065>
use StringUtils.isBlank
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (lines 1259 - 1260)
<https://reviews.apache.org/r/41916/#comment180067>
minor nit: String queueName = getLifecycleRetentionQueue(feed, feedCluster.getName()); - init in same line
common/src/main/java/org/apache/falcon/entity/FeedHelper.java (line 1261)
<https://reviews.apache.org/r/41916/#comment180066>
same as above
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 554)
<https://reviews.apache.org/r/41916/#comment180069>
nit: Use constants
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 555)
<https://reviews.apache.org/r/41916/#comment180070>
use StringUtils.isBlank
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 559)
<https://reviews.apache.org/r/41916/#comment180071>
nit: use strignUtils.isNotBlank
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (lines 567 - 569)
<https://reviews.apache.org/r/41916/#comment180072>
can we use LOG.info(" {}", ) instead of format
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 587)
<https://reviews.apache.org/r/41916/#comment180086>
use StringUtils
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java (line 588)
<https://reviews.apache.org/r/41916/#comment180087>
Since queueList is set, contains check not required.
common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java (line 355)
<https://reviews.apache.org/r/41916/#comment180088>
use LOG.info({}, ) instead of string format
- Sowmya Ramesh
On Feb. 10, 2016, 10:32 p.m., Venkatesan Ramachandran wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41916/
> -----------------------------------------------------------
>
> (Updated Feb. 10, 2016, 10:32 p.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 b3aaaab
> common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
> common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
> common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
> common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
> common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
> common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
>
> Diff: https://reviews.apache.org/r/41916/diff/
>
>
> Testing
> -------
>
> Unit tests
> Manual end to end tests
> Secure cluster test
>
>
> Thanks,
>
> Venkatesan Ramachandran
>
>
Re: Review Request 41916: FALCON-1584 : Falcon allows invalid hadoop
queue name for schedulable feed entities
Posted by Venkatesan Ramachandran <me...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41916/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 10:32 p.m.)
Review request for Falcon.
Changes
-------
+ Feed entity - check replication and retention queues
+ Support for Process Entity
+ Review board comments
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 (updated)
-----
common/src/main/java/org/apache/falcon/entity/FeedHelper.java b3aaaab
common/src/main/java/org/apache/falcon/entity/parser/FeedEntityParser.java 981e730
common/src/main/java/org/apache/falcon/entity/parser/ProcessEntityParser.java 10a5265
common/src/main/java/org/apache/falcon/util/HadoopQueueUtil.java PRE-CREATION
common/src/test/java/org/apache/falcon/entity/FeedHelperTest.java 95d10c4
common/src/test/java/org/apache/falcon/util/HadoopQueueUtilTest.java PRE-CREATION
common/src/test/resources/config/feed/feed-schedulerinfo-1.json PRE-CREATION
common/src/test/resources/config/feed/feed-schedulerinfo-2.json PRE-CREATION
Diff: https://reviews.apache.org/r/41916/diff/
Testing
-------
Unit tests
Manual end to end tests
Secure cluster test
Thanks,
Venkatesan Ramachandran