----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3018/#review3636 -----------------------------------------------------------
Some code formatting and error message comments. The test coverage has to be improved. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8119> Can you add a comment about what is being done in the nested if block below? trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8110> Please use braces for the if. Its extremely dangerous to have floating conditionals. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8114> Can the el.getChild("input-events", ns) be moved before the first if to avoid repeated calls to perform the same operation? Also applies to getChild("data-in", ns) trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8111> Can the el.getChild("input-events", ns) be moved before the first if to avoid repeated calls to perform the same operation? trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8112> Use one instead of 1 trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8120> Can you add a comment about what is being done in the nested if block below? trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8113> Please use braces for the if. Its extremely dangerous to have floating conditionals. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8115> Can the el.getChild("output-events", ns) be moved before the first if to avoid repeated calls to perform the same operation? Also applies to getChild("data-out", ns) trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8117> Change 1 to one. Is there a corrective action similar to that of the input-events? trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8124> The test case covers both input-events and output-events. Since input-events occurs first, the test case is not testing output-events. Can you add a separate test case for output-events and also test cases where input-events are specified with separate instance tags and similarly for output-events if its applicable. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8126> Can you also check for the error code? - Santhosh On 2011-12-05 20:15:04, Mona Chitnis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3018/ > ----------------------------------------------------------- > > (Updated 2011-12-05 20:15:04) > > > Review request for oozie, Mohammad Islam and Angelo K. Huang. > > > Summary > ------- > > For the coordinator input events, if there are multiple dates, it uses the > first date to resolve the data > instance, instead of giving warning or error. > For example: > <data-in name="din1" dataset="ds1"> > <instance>2010-06-11T00:30Z,2010-06-11T00:31Z,2010-06-11T00:32Z</instance> > </data-in> > > Similarly for output events. > > This patch will give error for the above case and protect against submission > of job that defaults to the first date without the knowledge of user. > > > This addresses bug OOZIE-15. > https://issues.apache.org/jira/browse/OOZIE-15 > > > Diffs > ----- > > > trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java > 1209756 > > trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java > 1209756 > > Diff: https://reviews.apache.org/r/3018/diff > > > Testing > ------- > > yes > > > Thanks, > > Mona > >
