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