> On Sept. 15, 2015, 8:59 p.m., Venkat Ranganathan wrote: > > common/src/main/java/org/apache/falcon/entity/parser/ClusterEntityParser.java, > > line 266 > > <https://reviews.apache.org/r/38391/diff/2/?file=1073948#file1073948line266> > > > > Any specific reason this is now protected? Is there an expectation to > > use it in derived classes in future? > > Balu Vellanki wrote: > It is now protected so that it can be accessed from unit tests.
That is good, but if these methods were private for a reason do we need to dilute the access. That is, would a public/protected method that encampasses the access to the private methods be the method to target in our tests and have the results of that method reflect the variations we want to test. It is more of a philosphical question as I see too many of the dilution of access for purposes of mock testing in a few projects. A thought for future patches :) - Venkat ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/38391/#review99162 ----------------------------------------------------------- On Sept. 15, 2015, 9:04 a.m., Balu Vellanki wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/38391/ > ----------------------------------------------------------- > > (Updated Sept. 15, 2015, 9:04 a.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 > >
