----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3018/#review3735 -----------------------------------------------------------
Some more comments. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8413> Please remove the initialization null. Objects are null by default. trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8411> For better code readability, please change the use of spec to eventsSpec, dataInSpec, instanceSpec. The tradeoff is between readability and the use of two more variables (8 local bytes) trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8409> I should have pointed this out earlier. Can this be rephrased to "input-events instance ' " + instance + " ' contains more than one date instance. Coordinator ... trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8412> For better code readability, please change the use of spec to eventsSpec, dataOutSpec, instanceSpec. The tradeoff is between readability and the use of one more variable (4 local bytes) trunk/core/src/main/java/org/apache/oozie/command/coord/CoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8410> I should have pointed this out earlier. Can this be rephrased to "output-events instance ' " + instance + " ' contains more than one date instance. Coordinator ... trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8420> The comment contents have to be changed to reflect the test case. Please remove the references to data-out. Add a note on the negative and positive test cases. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8414> Can the message be refined to state "Expected to catch errors due to incorrectly specified input data set instances" trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8417> In addition of the error code check can you add a check for the error message string. The error code for input and output instances is identical. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8415> Can the failure message be changed to "Unexpected failure: " + e" trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8419> Is there an existing test case for the positive case, i.e., a single output instance? If it exists then ignore this comment. Also, please add a comment on the test case similar to that of the previous test case, including the review comment feedback. trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8416> Can the message be refined to state "Expected to catch errors due to incorrectly specified output data set instances" trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java <https://reviews.apache.org/r/3018/#comment8418> In addition of the error code check can you add a check for the error message string. The error code for input and output instances is identical. - Santhosh On 2011-12-08 04:13:55, Mona Chitnis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3018/ > ----------------------------------------------------------- > > (Updated 2011-12-08 04:13:55) > > > 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 > >
