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

Switch to Plain View
Sqoop, mail # dev - Review Request: Review request for SQOOP-1033 "CombineFileInputFormat does not work with paths not on default FS like ASV"


+
Shuaishuai Nie 2013-05-08, 00:38
+
Venkat Ranganathan 2013-05-08, 16:00
+
Shuaishuai Nie 2013-05-08, 17:10
+
Shuaishuai Nie 2013-05-08, 17:11
+
Shuaishuai Nie 2013-05-08, 20:12
+
Shuaishuai Nie 2013-05-09, 22:16
+
Venkat Ranganathan 2013-05-10, 04:49
+
Shuaishuai Nie 2013-05-10, 14:51
+
Jarek Cecho 2013-05-20, 14:27
+
Venkat Ranganathan 2013-05-09, 21:57
+
Shuaishuai Nie 2013-05-08, 00:30
Copy link to this message
-
Re: Review Request: Review request for SQOOP-1035 "Add MS Sqoop Connecter tests to repo"
Venkat Ranganathan 2013-05-12, 15:48

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10987/#review20456
-----------------------------------------------------------
Patch looks good except for a few nits.  It is a big patch and adds lots of tests which is badly needed.    There are few places where the editor generated TODO comments have to be removed and the exception handling can be a little bit more precise (instead of printing the stack trace and ignoring the exception), the exceptions can be rethrown after printing the stackTrace for debugging purposes if debug mode is enabled (you can check the logger)

Similar issues on test setup.  If the setup throws an exception during one of the methods (say populate the table), it does not handle it -  rethrow to abort the test.  Similarly getConnection call ignores exception

Thanks

Venkat
src/test/com/cloudera/sqoop/hive/TestHiveImport.java
<https://reviews.apache.org/r/10987/#comment42139>

    Nit - spaces

src/test/org/apache/sqoop/manager/MSSQLTestDataFileParser.java
<https://reviews.apache.org/r/10987/#comment42140>

    Can you please remove the comment - it has been implemented

src/test/org/apache/sqoop/manager/MSSQLTestDataFileParser.java
<https://reviews.apache.org/r/10987/#comment42141>

    please remove this

src/test/org/apache/sqoop/manager/MSSQLTestUtils.java
<https://reviews.apache.org/r/10987/#comment42142>

    Please remove this.   Don't we want to return null in this case?

src/test/org/apache/sqoop/manager/MSSQLTestUtils.java
<https://reviews.apache.org/r/10987/#comment42143>

    Please remove this exception block.  As the method is throwing the SQLException, it should be OK and the caller can handle the exception

src/test/org/apache/sqoop/manager/MSSQLTestUtils.java
<https://reviews.apache.org/r/10987/#comment42144>

    Please remove this block or rethrow the exception

src/test/org/apache/sqoop/manager/MSSQLTestUtils.java
<https://reviews.apache.org/r/10987/#comment42145>

    Same as before.   There are too many of these in the files, can you please address all of them.
- Venkat Ranganathan
On May 8, 2013, 12:30 a.m., Shuaishuai Nie wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10987/
> -----------------------------------------------------------
>
> (Updated May 8, 2013, 12:30 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Add the MS Sqoop connector tests that test integration scenarios with SQL Server to the repo.
>
>
> This addresses bug SQOOP-1035.
>     https://issues.apache.org/jira/browse/SQOOP-1035
>
>
> Diffs
> -----
>
>   conf/MSTest.properties PRE-CREATION
>   src/test/com/cloudera/sqoop/hive/TestHiveImport.java 462ccf1
>   src/test/com/cloudera/sqoop/manager/SQLServerManagerImportManualTest.java 27860c2
>   src/test/org/apache/sqoop/manager/MSSQLTestData.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/MSSQLTestDataFileParser.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/MSSQLTestUtils.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/ManagerCompatExport.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerDatatypeExportDelimitedFileManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerDatatypeExportSequenceFileManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerDatatypeImportDelimitedFileManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerDatatypeImportSequenceFileManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerHiveImportManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerManagerManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerMultiColsManualTest.java PRE-CREATION
>   src/test/org/apache/sqoop/manager/SQLServerMultiMapsManualTest.java PRE-CREATION
+
Shuaishuai Nie 2013-05-13, 20:54
+
Shuaishuai Nie 2013-05-14, 00:31
+
Shuaishuai Nie 2013-05-16, 18:48
+
Venkat Ranganathan 2013-05-21, 20:03
+
Shuaishuai Nie 2013-05-23, 17:55
+
Venkat Ranganathan 2013-05-25, 00:37
+
Abhijeet Gaikwad 2013-06-03, 10:36
+
Jarek Cecho 2013-06-09, 16:02
+
Shuaishuai Nie 2013-06-12, 17:30
+
Jarek Cecho 2013-06-09, 16:23
+
Shuaishuai Nie 2013-06-12, 17:30
+
Shuaishuai Nie 2013-06-12, 17:30
+
Venkat Ranganathan 2013-06-12, 17:51
+
Jarek Cecho 2013-06-15, 17:41
+
Shuaishuai Nie 2013-06-18, 20:57
+
Shuaishuai Nie 2013-06-18, 20:57
+
Jarek Cecho 2013-06-18, 22:09
+
Shuaishuai Nie 2013-06-18, 22:24
+
Shuaishuai Nie 2013-05-21, 21:51
+
Shuaishuai Nie 2013-05-21, 21:10
+
Shuaishuai Nie 2013-05-22, 16:59
+
Shuaishuai Nie 2013-05-30, 17:11
+
Venkat Ranganathan 2013-05-30, 17:49
+
Venkat Ranganathan 2013-05-30, 06:55
+
Venkat Ranganathan 2013-05-22, 06:48
+
Raghav Gautam 2013-05-31, 20:37
+
Jarek Cecho 2013-06-02, 14:07
+
Raghav Gautam 2013-06-06, 03:56
+
Raghav Gautam 2013-06-06, 03:56
+
Jarek Cecho 2013-06-09, 15:17
+
Raghav Gautam 2013-06-10, 21:29
+
Venkat Ranganathan 2013-06-10, 23:32
+
Jarek Cecho 2013-06-11, 18:54