sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Boglarka Egyed <egyedb...@gmail.com>
Subject Re: Review Request 55079: [SQOOP-3096] Add management of the schema in direct netezza import
Date Mon, 02 Jan 2017 15:10:13 GMT

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



Hi Frédéric,

I have added some review to the specific lines, most of them related to indentation problems.
On the top of them I would have two more things:

1. Could you please recursively replace every usage of the DBConfiguration's getInputTableName()
in Netezza related classes, e.g. in NetezzaDataDrivenDBInputFormat too?

2. Could you please add some unit test cases for this new command line option? 

Thanks,
Bogi


src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (lines 299 - 301)
<https://reviews.apache.org/r/55079/#comment231473>

    These lines have an incorrect indentation, please correct it.



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (lines 311 - 312)
<https://reviews.apache.org/r/55079/#comment231474>

    Is this included into the log intentionally or was just because of debugging reasons?
If it's intentional, could you please provide a more verbose description instead of just writing
"extraArgs"?



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (lines 331 - 336)
<https://reviews.apache.org/r/55079/#comment231475>

    These lines have an incorrect indentation, please correct them.



src/java/org/apache/sqoop/manager/DirectNetezzaManager.java (lines 370 - 383)
<https://reviews.apache.org/r/55079/#comment231477>

    These lines have an incorrect indentation, please correct them.



src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java (line
81)
<https://reviews.apache.org/r/55079/#comment231480>

    In escapeTableName method of the DirectNetezzaManager class you checked got schema value
against null and isEmpty too. Is there any specific reason for checking it here only against
null value?


- Boglarka Egyed


On Jan. 1, 2017, 8:33 p.m., Frédéric Escandell wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55079/
> -----------------------------------------------------------
> 
> (Updated Jan. 1, 2017, 8:33 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> SQOOP-3096
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java af15824 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java
2efea53 
> 
> Diff: https://reviews.apache.org/r/55079/diff/
> 
> 
> Testing
> -------
> 
> Tested with a schema included in Netezza database (adding -- --schema <schema name>)
to the sqoop import command line
> 
> 
> Thanks,
> 
> Frédéric Escandell
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message