You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Jarek Cecho <ja...@apache.org> on 2012/10/01 02:19:41 UTC
Review Request: SQOOP-540: Microsoft SQL Connector doesn't support custom
schemas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7369/
-----------------------------------------------------------
Review request for Sqoop.
Summary (updated)
-----------------
SQOOP-540: Microsoft SQL Connector doesn't support custom schemas
Description (updated)
-------
I've chosen similar implementation path as was used in PostgreSQL Connector.
This addresses bug SQOOP-540.
https://issues.apache.org/jira/browse/SQOOP-540
Diffs
-----
src/java/org/apache/sqoop/manager/SQLServerManager.java 7ce1edd
src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java a56b93d
src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java PRE-CREATION
src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 0f949f4
Diff: https://reviews.apache.org/r/7369/diff/
Testing (updated)
-------
I've test my changes (both import and export) against Microsoft SQL Server Express 2012.
Thanks,
Jarek Cecho
Re: Review Request: SQOOP-540: Microsoft SQL Connector doesn't support
custom schemas
Posted by Jarek Cecho <ja...@apache.org>.
> On Oct. 1, 2012, 6:39 p.m., Cheolsoo Park wrote:
> > Looks good. I have one question regarding CONNECTOR_FACTORY as below.
> >
> > I also found "ant checkstyle" fails with an error. Although the error is not introduced by this patch, can you please address it in this patch too?
> >
> > - if((Boolean)isSecurityEnabled.invoke(null)) {
> > + if ((Boolean)isSecurityEnabled.invoke(null)) {
> >
> > Thanks!
Thank you very much for your input sir. I appreciate your time.
> On Oct. 1, 2012, 6:39 p.m., Cheolsoo Park wrote:
> > src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java, line 85
> > <https://reviews.apache.org/r/7369/diff/1/?file=167761#file167761line85>
> >
> > This was added to make it easier to test the MS connector (http://www.microsoft.com/en-us/download/details.aspx?id=27584). Are you removing this because testing the MS connector is not that useful anymore?
> >
> > If that's the intention, can you please remove relevant code in build.xml (line 261 - 271) too?
I've actually misunderstand the purpose of this part of the code and removed it under impressions that it's outdated. I'll put it back and I'll also add it to newly introduced Export test case.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7369/#review12071
-----------------------------------------------------------
On Oct. 1, 2012, 12:19 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7369/
> -----------------------------------------------------------
>
> (Updated Oct. 1, 2012, 12:19 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've chosen similar implementation path as was used in PostgreSQL Connector.
>
>
> This addresses bug SQOOP-540.
> https://issues.apache.org/jira/browse/SQOOP-540
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/SQLServerManager.java 7ce1edd
> src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java a56b93d
> src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java PRE-CREATION
> src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 0f949f4
>
> Diff: https://reviews.apache.org/r/7369/diff/
>
>
> Testing
> -------
>
> I've test my changes (both import and export) against Microsoft SQL Server Express 2012.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-540: Microsoft SQL Connector doesn't support
custom schemas
Posted by Cheolsoo Park <ch...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7369/#review12071
-----------------------------------------------------------
Looks good. I have one question regarding CONNECTOR_FACTORY as below.
I also found "ant checkstyle" fails with an error. Although the error is not introduced by this patch, can you please address it in this patch too?
- if((Boolean)isSecurityEnabled.invoke(null)) {
+ if ((Boolean)isSecurityEnabled.invoke(null)) {
Thanks!
src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java
<https://reviews.apache.org/r/7369/#comment25722>
This was added to make it easier to test the MS connector (http://www.microsoft.com/en-us/download/details.aspx?id=27584). Are you removing this because testing the MS connector is not that useful anymore?
If that's the intention, can you please remove relevant code in build.xml (line 261 - 271) too?
- Cheolsoo Park
On Oct. 1, 2012, 12:19 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7369/
> -----------------------------------------------------------
>
> (Updated Oct. 1, 2012, 12:19 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've chosen similar implementation path as was used in PostgreSQL Connector.
>
>
> This addresses bug SQOOP-540.
> https://issues.apache.org/jira/browse/SQOOP-540
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/SQLServerManager.java 7ce1edd
> src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java a56b93d
> src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java PRE-CREATION
> src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 0f949f4
>
> Diff: https://reviews.apache.org/r/7369/diff/
>
>
> Testing
> -------
>
> I've test my changes (both import and export) against Microsoft SQL Server Express 2012.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-540: Microsoft SQL Connector doesn't support
custom schemas
Posted by Cheolsoo Park <ch...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7369/#review12092
-----------------------------------------------------------
Ship it!
Ship It!
- Cheolsoo Park
On Oct. 1, 2012, 10:18 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7369/
> -----------------------------------------------------------
>
> (Updated Oct. 1, 2012, 10:18 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've chosen similar implementation path as was used in PostgreSQL Connector.
>
>
> This addresses bug SQOOP-540.
> https://issues.apache.org/jira/browse/SQOOP-540
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/manager/SQLServerManager.java 7ce1edd
> src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java a6e5546
> src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java a56b93d
> src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java PRE-CREATION
> src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 0f949f4
>
> Diff: https://reviews.apache.org/r/7369/diff/
>
>
> Testing
> -------
>
> I've test my changes (both import and export) against Microsoft SQL Server Express 2012.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-540: Microsoft SQL Connector doesn't support
custom schemas
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7369/
-----------------------------------------------------------
(Updated Oct. 1, 2012, 10:18 p.m.)
Review request for Sqoop.
Changes
-------
I've incorporated Cheolsoo's comments.
Description
-------
I've chosen similar implementation path as was used in PostgreSQL Connector.
This addresses bug SQOOP-540.
https://issues.apache.org/jira/browse/SQOOP-540
Diffs (updated)
-----
src/java/org/apache/sqoop/manager/SQLServerManager.java 7ce1edd
src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java a6e5546
src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java a56b93d
src/test/com/cloudera/sqoop/manager/SQLServerManagerExportManualTest.java PRE-CREATION
src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 0f949f4
Diff: https://reviews.apache.org/r/7369/diff/
Testing
-------
I've test my changes (both import and export) against Microsoft SQL Server Express 2012.
Thanks,
Jarek Cecho