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



Hi Alex,

I can still see some reformats in the diffs, can you please fix those as well? 
Apart from that when I tried to apply the patch locally I got the following 
warnings, please address these as well:

/Users/szabolcsvasas/Downloads/DirectPostgresqlManager_v2.patch:312: trailing 
whitespace.
  private CopyManager getPostgresCopyAPI(ImportJobContext context) 
/Users/szabolcsvasas/Downloads/DirectPostgresqlManager_v2.patch:314: trailing 
whitespace.
      
/Users/szabolcsvasas/Downloads/DirectPostgresqlManager_v2.patch:322: trailing 
whitespace.
      
/Users/szabolcsvasas/Downloads/DirectPostgresqlManager_v2.patch:324: trailing 
whitespace.
        conn = dbConf.getConnection(); 
/Users/szabolcsvasas/Downloads/DirectPostgresqlManager_v2.patch:335: trailing 
whitespace.
  
warning: squelched 8 whitespace errors
warning: 13 lines add whitespace errors.

I have also replied to some of your comments below.

- Szabolcs Vasas


On June 30, 2017, 5:17 p.m., Alex Ang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59710/
> -----------------------------------------------------------
> 
> (Updated June 30, 2017, 5:17 p.m.)
> 
> 
> Review request for Sqoop and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-3190
>     https://issues.apache.org/jira/browse/SQOOP-3190
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> Current Postgres direct import requires the installation of the psql utility 
> on all hadoop nodes. However, due to system constraints, this may not always 
> be possible. 
> It should be noted that Postgres provides a Java API which can execute the 
> same copy command as the psql utility. As such, it would be more ideal to 
> remove the psql utility dependency and switch to the Postgres Java API.
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/manager/DirectPostgresqlManager.java 63b0704 
> 
> 
> Diff: https://reviews.apache.org/r/59710/diff/2/
> 
> 
> Testing
> -------
> 
> Tested with PostgresqlImportTest and passed all tests.
> 
> 
> Thanks,
> 
> Alex Ang
> 
>

Reply via email to