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

Switch to Plain View
Sqoop, mail # dev - Review Request: SQOOP-918 Sqoop2: Introduce client API and change Sqoop shell to use it


+
Jarek Cecho 2013-03-03, 02:13
+
Jarek Cecho 2013-03-10, 17:20
+
Jarek Cecho 2013-03-11, 00:04
+
Jarek Cecho 2013-03-11, 02:21
+
Cheolsoo Park 2013-03-11, 04:28
+
Cheolsoo Park 2013-03-11, 02:03
+
Jarek Cecho 2013-03-11, 02:23
+
Cheolsoo Park 2013-03-10, 07:02
+
Jarek Cecho 2013-03-10, 17:10
+
Jarek Cecho 2013-03-10, 15:17
+
Cheolsoo Park 2013-03-10, 22:55
Copy link to this message
-
Re: Review Request: SQOOP-918 Sqoop2: Introduce client API and change Sqoop shell to use it
Jarek Cecho 2013-03-11, 00:00


> On March 10, 2013, 7:02 a.m., Cheolsoo Park wrote:
> > client/src/main/java/org/apache/sqoop/client/request/SqoopRequests.java, lines 129-140
> > <https://reviews.apache.org/r/9717/diff/1/?file=264575#file264575line129>
> >
> >     Can you factor out this into a separate method so that duplicate code can be removed in createConnectionApplyValidations and updateConnectionApplyValidations?
> >
>
> Jarek Cecho wrote:
>     I would prefer to do it once SQOOP-919 will get committed, so that I can use the newly introduced MFormList list class during refactorization. Would be follow up JIRA acceptable for you at this point?
>
> Cheolsoo Park wrote:
>     Sure. If you're planning to do it later, that's fine.

Filled as SQOOP-942.
> On March 10, 2013, 7:02 a.m., Cheolsoo Park wrote:
> > client/src/main/java/org/apache/sqoop/client/request/SqoopRequests.java, lines 179-190
> > <https://reviews.apache.org/r/9717/diff/1/?file=264575#file264575line179>
> >
> >     The same here as above. Can you factor out this to remove duplicate code?
>
> Jarek Cecho wrote:
>     I would prefer to do it once SQOOP-919 will get committed, so that I can use the newly introduced MFormList list class during refactorization. Would be follow up JIRA acceptable for you at this point?
>
> Cheolsoo Park wrote:
>     Sure. If you're planning to do it later, that's fine.

Filled as SQOOP-942.
> On March 10, 2013, 7:02 a.m., Cheolsoo Park wrote:
> > client/src/main/java/org/apache/sqoop/client/SqoopClient.java, line 255
> > <https://reviews.apache.org/r/9717/diff/1/?file=264569#file264569line255>
> >
> >     Wouldn't "getSubmissionStatus" be a better name for this method?
>
> Jarek Cecho wrote:
>     I feel that it would be incompatible with the other method names that are in form "action" "object", e.g. "startSubmission", "updateJob", "createConnection". That being said, I can definitely change it if you prefer to, I personally prefer this way.
>
> Cheolsoo Park wrote:
>     I agree that it would be good to keep the form of "(action)(object)". But "action" has to be a verb. "statusSubmission" doesn't make sense to me because "status" is not an action. If you could replace "status" with a verb, that would be great.
>    
>     I have thought about it but didn't have a good idea. So I suggested "getSubmissionStatus".

Good point, I buy that. I'll upload refreshed patch shortly.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9717/#review17654
-----------------------------------------------------------
On March 10, 2013, 5:20 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9717/
> -----------------------------------------------------------
>
> (Updated March 10, 2013, 5:20 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've created first simple SqoopClient API and altered shell to use it. I've noticed that there is a lot of duplicate code in the shell, so I've tried to clean it up. The changes are simple, however are spanning across all shell files, thus making this patch bigger than necessary.
>
> This is first implementation of the SqoopClient API. I'm intending to improve it, for example by caching objects that was already received (especially connector info and resource bundles). As this patch is big enough already, I would prefer to finish this functionality in follow up JIRA.
>
>
> This addresses bug SQOOP-918.
>     https://issues.apache.org/jira/browse/SQOOP-918
>
>
> Diffs
> -----
>
>   client/src/main/java/org/apache/sqoop/client/SqoopClient.java PRE-CREATION
>   client/src/main/java/org/apache/sqoop/client/core/Constants.java c364aa8856bf02f501b0ca2f3630c8e8c6fd10b1
>   client/src/main/java/org/apache/sqoop/client/core/Environment.java 5d1af26a0e8f96c11c7ef7d8fcfd0013db5ccce6