Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Plain View
Sqoop, mail # dev - Review Request:  Sqoop2: Introduce synchronous job submission to Client API


+
rj.vasanthkumar@... 2013-04-30, 19:56
+
Jarek Cecho 2013-05-02, 00:49
+
rj.vasanthkumar@... 2013-05-02, 14:43
+
Jarek Cecho 2013-05-03, 05:08
+
rj.vasanthkumar@... 2013-05-03, 13:18
+
rj.vasanthkumar@... 2013-05-03, 20:31
+
rj.vasanthkumar@... 2013-05-03, 22:14
+
Jarek Cecho 2013-05-20, 13:59
Copy link to this message
-
Re: Review Request: Sqoop2: Introduce synchronous job submission to Client API
Jarek Cecho 2013-05-03, 21:05

-----------------------------------------------------------
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
>
>

+
Jarek Cecho 2013-05-03, 14:18