-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10869/#review20147
-----------------------------------------------------------


Hi Vasanth,
thank you for incorporating all my comments. I do have couple of more:


client/src/main/java/org/apache/sqoop/client/SqoopClient.java
<https://reviews.apache.org/r/10869/#comment41397>

    Please use CamelCase for naming the Enum, so that we're consistent with 
rest of the code base.



client/src/main/java/org/apache/sqoop/client/SqoopClient.java
<https://reviews.apache.org/r/10869/#comment41393>

    I'm thinking what will happen when the pollTime will be zero? Maybe it 
would be worth changing the condition to pollTime <= 0?



client/src/main/java/org/apache/sqoop/client/SqoopClient.java
<https://reviews.apache.org/r/10869/#comment41395>

    By supplying the pollTime parameter user is effectively driving how many 
HTTP request will be generated or what will be the minimal delay of getting 
update from Server. Having said that I think we should not override it even 
when user is not interested in getting any callbacks called.



client/src/main/java/org/apache/sqoop/client/SqoopClient.java
<https://reviews.apache.org/r/10869/#comment41401>

    Nit: "} else {"



client/src/main/java/org/apache/sqoop/client/SqoopClient.java
<https://reviews.apache.org/r/10869/#comment41399>

    Can you provide Java doc to this method describing what this method does?
    
    Also I would suggest to rename the method as it do not seem to be setting 
any submission status. Perhaps submissionCallback()?


- Jarek Cecho


On May 3, 2013, 8:31 p.m., vasanthkumar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10869/
> -----------------------------------------------------------
> 
> (Updated May 3, 2013, 8:31 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Description
> -------
> 
> Synchronous job submission to Client API.
> 
> 
> This addresses bug SQOOP-985.
>     https://issues.apache.org/jira/browse/SQOOP-985
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/sqoop/client/SqoopClient.java f9137bb 
>   client/src/main/java/org/apache/sqoop/client/SubmissionCallback.java 
> PRE-CREATION 
>   client/src/main/java/org/apache/sqoop/client/core/ClientError.java 179dc55 
>   
> client/src/main/java/org/apache/sqoop/client/shell/SubmissionStartFunction.java
>  f68ac11 
> 
> Diff: https://reviews.apache.org/r/10869/diff/
> 
> 
> Testing
> -------
> 
> Done
> 
> 
> Thanks,
> 
> vasanthkumar
> 
>

Reply via email to