----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41845/#review112524 -----------------------------------------------------------
client/src/main/java/org/apache/falcon/client/FalconClient.java (line 595) <https://reviews.apache.org/r/41845/#comment173010> Shouldn't we check here for a valid date format instead of just checking for null? It will serve as an early sanity check at the client side without hitting the server. client/src/main/java/org/apache/falcon/client/FalconClient.java (line 940) <https://reviews.apache.org/r/41845/#comment173013> Isn't "start" already added as part of the addParamsToResource(..) method call above (in line 932)? client/src/main/java/org/apache/falcon/client/FalconClient.java (line 941) <https://reviews.apache.org/r/41845/#comment173008> I think this will cause checkstyle issue. You need to put space after the comma. In general you can check the sanity of your patch by running mvn clean install -Ptest-patch,hadoop-2 before submitting the patch. prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java (line 545) <https://reviews.apache.org/r/41845/#comment173014> I am guessing there is already a validation for start to be in valid date format, won't that handle this also? If there is no such validation then we should add that validation and check for null/invalid format - Ajay Yadava On Jan. 2, 2016, 2:43 p.m., Praveen Adlakha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41845/ > ----------------------------------------------------------- > > (Updated Jan. 2, 2016, 2:43 p.m.) > > > Review request for Falcon. > > > Bugs: Falcon-1708 > https://issues.apache.org/jira/browse/Falcon-1708 > > > Repository: falcon-git > > > Description > ------- > > params API does not take start as a mandatory option > > > Diffs > ----- > > client/src/main/java/org/apache/falcon/client/FalconClient.java 4a773c6 > prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java > d50d111 > > Diff: https://reviews.apache.org/r/41845/diff/ > > > Testing > ------- > > dataqa@blr-test-129:/usr/local/falcon/falcon-0.9-SNAPSHOT$ bin/falcon > instance -type process -name DWH-CI-daily-supply2 -params > ERROR: start date is mandatory and should not be null > dataqa@blr-test-129:/usr/local/falcon/falcon-0.9-SNAPSHOT$ bin/falcon > instance -type process -name DWH-CI-daily-supply2 -params -start > 2015-12-30T01:00Z > Consolidated Status: SUCCEEDED > > Instances: > Instance Cluster SourceCluster Status > Start End Details Log > ----------------------------------------------------------------------------------------------- > 2015-12-30T01:00Z local - WAITING - - > hdfs://10.14.118.114:8020/user/dataqa/testEffective/2015/12/29/23/00 - > > Additional Information: > Response: default/PARAMS > Request Id: default/2059794500@qtp-1718798991-9 - > 2204721d-3ec8-4eac-b344-1ead60a57e0d > > > Thanks, > > Praveen Adlakha > >
