Re: Review Request 67929: Remove Kite dependency from the Sqoop project

2018-07-19 Thread daniel voros 

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


Ship it!




Thanks for the update! Verified on same cluster. Ship it!

- daniel voros


On July 19, 2018, 1:52 p.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67929/
> ---
> 
> (Updated July 19, 2018, 1:52 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3329
> https://issues.apache.org/jira/browse/SQOOP-3329
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> - Removed kitesdk dependency from ivy.xml
> - Removed Kite Dataset API based Parquet import implementation
> - Since Parquet library was a transitive dependency of the Kite SDK I added 
> org.apache.parquet.avro-parquet 1.9 as a direct dependency
> - In this dependency the parquet package has changed to org.apache.parquet so 
> I needed to make changes in several classes according to this
> - Removed all the Parquet related test cases from TestHiveImport. These 
> scenarios are already covered in TestHiveServer2ParquetImport.
> - Modified the documentation to reflect these changes.
> 
> 
> Diffs
> -
> 
>   ivy.xml 1f587f3eb 
>   ivy/libraries.properties 565a8bf50 
>   src/docs/user/hive-notes.txt af97d94b3 
>   src/docs/user/import.txt a2c16d956 
>   src/java/org/apache/sqoop/SqoopOptions.java cc1b75281 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/ParquetJobConfiguratorImplementation.java
>  050c85488 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteMergeParquetReducer.java 
> 02816d77f 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetExportJobConfigurator.java
>  6ebc5a31b 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetExportMapper.java 
> 122ff3fc9 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetImportJobConfigurator.java
>  7e179a27d 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetImportMapper.java 
> 0a91e4a20 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetJobConfiguratorFactory.java
>  bd07c09f4 
>   
> src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetMergeJobConfigurator.java
>  ed045cd14 
>   src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetUtils.java 
> a4768c932 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 87fc5e987 
>   src/test/org/apache/sqoop/TestMerge.java 2b3280a5a 
>   src/test/org/apache/sqoop/TestParquetExport.java 0fab1880c 
>   src/test/org/apache/sqoop/TestParquetImport.java b1488e8af 
>   src/test/org/apache/sqoop/hive/TestHiveImport.java 436f0e512 
>   src/test/org/apache/sqoop/tool/TestBaseSqoopTool.java dbda8b7f4 
> 
> 
> Diff: https://reviews.apache.org/r/67929/diff/2/
> 
> 
> Testing
> ---
> 
> Ran unit and third party tests.
> 
> 
> File Attachments
> 
> 
> trunkdependencies.graphml
>   
> https://reviews.apache.org/media/uploaded/files/2018/07/18/4df23fec-c7a7-4dc6-8ac1-0872ee6fdadf__trunkdependencies.graphml
> kiteremovaldependencies.graphml
>   
> https://reviews.apache.org/media/uploaded/files/2018/07/18/e8cbb4d3-1da3-4b64-96ea-09f647ece126__kiteremovaldependencies.graphml
> 
> 
> Thanks,
> 
> Szabolcs Vasas
> 
>

Re: Review Request 62492: SQOOP-3224: Mainframe FTP transfer should have an option to use binary mode for transfer

2018-07-19 Thread Boglarka Egyed 

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



Hi Chris,

I took a look at the current version of your patch and had several findings, 
please find them below. Unfortunately I couldn't get through the whole diff, I 
will iterate over it again later.

Also, applying your patch throws warnings:

/Users/boglarkaegyed/Downloads/SQOOP-3224-25.patch:52: trailing whitespace.
In the case of generation data group datasets, Sqoop will retrieve just the 
last or 
/Users/boglarkaegyed/Downloads/SQOOP-3224-25.patch:61: trailing whitespace.
Support for binary datasets by specifying --as-binaryfile and optionally 
--buffersize followed by 
/Users/boglarkaegyed/Downloads/SQOOP-3224-25.patch:63: trailing whitespace.
will alter the number of records Sqoop reports to have imported. This is 
because it reads the 
/Users/boglarkaegyed/Downloads/SQOOP-3224-25.patch:73: trailing whitespace.
Import of a tape based generation data group dataset using a password alias and 
writing out to 
/Users/boglarkaegyed/Downloads/SQOOP-3224-25.patch:81: trailing whitespace.
Import of a tape based binary generation data group dataset with a buffer size 
of 64000 using a 
warning: squelched 1 whitespace error
warning: 6 lines add whitespace errors.

Could you please correct these?

Thanks,
Bogi


src/docs/user/import-mainframe.txt
Lines 197 (patched)


What does the "-" character mean at the end in this line? Also, could you 
please use the word "option" instead of "parameter" here?



src/java/org/apache/sqoop/mapreduce/mainframe/AbstractMainframeDatasetImportMapper.java
Lines 35 (patched)


There is no logging made in the code currently, LOG is unused.



src/java/org/apache/sqoop/mapreduce/mainframe/AbstractMainframeDatasetImportMapper.java
Lines 39 (patched)


Could you please use a more self-explanatory name here?



src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetBinaryRecord.java
Lines 41 (patched)


There is no logging made in the code currently, LOG is unused.



src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetFTPRecordReader.java
Lines 59-62 (patched)


Input parameters inputSplit and taskAttemptContext are unused here and 
InterruptedException seems to be unnecessary too.



src/java/org/apache/sqoop/mapreduce/mainframe/MainframeImportJob.java
Lines 123 (patched)


ClassNotFoundException seems to be unnecessary here.



src/java/org/apache/sqoop/mapreduce/mainframe/MainframeImportJob.java
Lines 124 (patched)


In line 77 and 108 you used equals method instead of ==, could you please 
stay consistent?



src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetBinaryRecord.java
Lines 41 (patched)


In general I suggest to use full names instead of abbreviations for better 
understanding.



src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetBinaryRecord.java
Lines 46-48 (patched)


These could be local variables.



src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetBinaryRecord.java
Lines 49-50 (patched)


These could be private.



src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetBinaryRecord.java
Lines 66-67 (patched)


Why don't you use it DATASET_NAME here instead of "dummy.ds"?



src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetBinaryRecord.java
Lines 79-81 (patched)


Variables bytes and len are unused here.



src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetBinaryRecord.java
Lines 98 (patched)


This could be simplified to assertEquals().



src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetBinaryRecord.java
Lines 136 (patched)


This could be simplified to assertEquals().



src/test/org/apache/sqoop/tool/TestMainframeImportTool.java
Lines 194-252 (patched)


Exception handling is not consistent here. You have 
org.apache.sqoop.SqoopOptions and 
org.apache.sqoop.SqoopOptiopns.InvalidOptionException imported too and yet you 
use the following expressions after "throws":
-

Re: Review Request 67929: Remove Kite dependency from the Sqoop project

2018-07-19 Thread Szabolcs Vasas 

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

(Updated July 19, 2018, 1:52 p.m.)


Review request for Sqoop.


Changes
---

Parquet version is set to 1.6.0.


Bugs: SQOOP-3329
https://issues.apache.org/jira/browse/SQOOP-3329


Repository: sqoop-trunk


Description
---

- Removed kitesdk dependency from ivy.xml
- Removed Kite Dataset API based Parquet import implementation
- Since Parquet library was a transitive dependency of the Kite SDK I added 
org.apache.parquet.avro-parquet 1.9 as a direct dependency
- In this dependency the parquet package has changed to org.apache.parquet so I 
needed to make changes in several classes according to this
- Removed all the Parquet related test cases from TestHiveImport. These 
scenarios are already covered in TestHiveServer2ParquetImport.
- Modified the documentation to reflect these changes.


Diffs (updated)
-

  ivy.xml 1f587f3eb 
  ivy/libraries.properties 565a8bf50 
  src/docs/user/hive-notes.txt af97d94b3 
  src/docs/user/import.txt a2c16d956 
  src/java/org/apache/sqoop/SqoopOptions.java cc1b75281 
  
src/java/org/apache/sqoop/mapreduce/parquet/ParquetJobConfiguratorImplementation.java
 050c85488 
  src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteMergeParquetReducer.java 
02816d77f 
  
src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetExportJobConfigurator.java
 6ebc5a31b 
  src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetExportMapper.java 
122ff3fc9 
  
src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetImportJobConfigurator.java
 7e179a27d 
  src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetImportMapper.java 
0a91e4a20 
  
src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetJobConfiguratorFactory.java
 bd07c09f4 
  
src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetMergeJobConfigurator.java
 ed045cd14 
  src/java/org/apache/sqoop/mapreduce/parquet/kite/KiteParquetUtils.java 
a4768c932 
  src/java/org/apache/sqoop/tool/BaseSqoopTool.java 87fc5e987 
  src/test/org/apache/sqoop/TestMerge.java 2b3280a5a 
  src/test/org/apache/sqoop/TestParquetExport.java 0fab1880c 
  src/test/org/apache/sqoop/TestParquetImport.java b1488e8af 
  src/test/org/apache/sqoop/hive/TestHiveImport.java 436f0e512 
  src/test/org/apache/sqoop/tool/TestBaseSqoopTool.java dbda8b7f4 


Diff: https://reviews.apache.org/r/67929/diff/2/

Changes: https://reviews.apache.org/r/67929/diff/1-2/


Testing
---

Ran unit and third party tests.


File Attachments


trunkdependencies.graphml
  
https://reviews.apache.org/media/uploaded/files/2018/07/18/4df23fec-c7a7-4dc6-8ac1-0872ee6fdadf__trunkdependencies.graphml
kiteremovaldependencies.graphml
  
https://reviews.apache.org/media/uploaded/files/2018/07/18/e8cbb4d3-1da3-4b64-96ea-09f647ece126__kiteremovaldependencies.graphml


Thanks,

Szabolcs Vasas

Re: Review Request 67971: SQOOP-3346: Upgrade Hadoop version to 2.8.0

2018-07-19 Thread daniel voros 

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


Ship it!




Ship It!

- daniel voros


On July 19, 2018, 9 a.m., Boglarka Egyed wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67971/
> ---
> 
> (Updated July 19, 2018, 9 a.m.)
> 
> 
> Review request for Sqoop, daniel voros, Fero Szabo, and Szabolcs Vasas.
> 
> 
> Bugs: SQOOP-3346
> https://issues.apache.org/jira/browse/SQOOP-3346
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> Upgrading Hadoop version from 2.6.0 to 2.8.0 and some related code changes.
> 
> 
> Diffs
> -
> 
>   ivy/libraries.properties 565a8bf50cdd88597a2a502d2fdbce2d5c8585ef 
>   src/java/org/apache/sqoop/config/ConfigurationConstants.java 
> 666852c2af2f7636bd068c24e5df32173b185603 
>   src/java/org/apache/sqoop/config/ConfigurationHelper.java 
> fb2ab031caef023dfbd8130814d07416dbf4db14 
>   src/java/org/apache/sqoop/mapreduce/JobBase.java 
> 6d1e04992c0e1d45a24e22fcd765c286e7414578 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> f7310b939a667e4434a78bdbc50f9520fe72f8a6 
>   src/test/org/apache/sqoop/TestSqoopOptions.java 
> ba4a4d44f36c155318092bdcc71588c476e84e2d 
>   src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsTest.java 
> 833ebe8a14e438daa7fbb2eae13dc0d04bec3bb8 
>   src/test/org/apache/sqoop/orm/TestParseMethods.java 
> 46bb52d562991bc9c3443b8a26c7a7f9996d72d2 
> 
> 
> Diff: https://reviews.apache.org/r/67971/diff/1/
> 
> 
> Testing
> ---
> 
> Ran unit and 3rd party tests successfully.
> 
> 
> Thanks,
> 
> Boglarka Egyed
> 
>

[jira] [Updated] (SQOOP-3346) Upgrade Hadoop version to 2.8.0

2018-07-19 Thread Boglarka Egyed (JIRA) 


 [ 
https://issues.apache.org/jira/browse/SQOOP-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Boglarka Egyed updated SQOOP-3346:
--
Attachment: SQOOP-3346.patch

> Upgrade Hadoop version to 2.8.0
> ---
>
> Key: SQOOP-3346
> URL: https://issues.apache.org/jira/browse/SQOOP-3346
> Project: Sqoop
>  Issue Type: Sub-task
>Reporter: Boglarka Egyed
>Assignee: Boglarka Egyed
>Priority: Major
> Attachments: SQOOP-3346.patch
>
>
> Support for AWS temporary credentials has been introduced in Hadoop 2.8.0 
> based on HADOOP-12537 and it would make more sense to test and support this 
> capability too with Sqoop.
> There is [SQOOP-3305|https://reviews.apache.org/r/66300/bugs/SQOOP-3305/] 
> being open for upgrading Hadoop to 3.0.0 however it has several issues 
> described in [https://reviews.apache.org/r/66300/] currently thus I would 
> like to proceed with an "intermediate" upgrade to 2.8.0 to enable development 
> on S3 front. [~dvoros] are you OK with this?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Commented] (SQOOP-3346) Upgrade Hadoop version to 2.8.0

2018-07-19 Thread Boglarka Egyed (JIRA) 


[ 
https://issues.apache.org/jira/browse/SQOOP-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16549027#comment-16549027
 ] 

Boglarka Egyed commented on SQOOP-3346:
---

Great, thank you [~dvoros]!

> Upgrade Hadoop version to 2.8.0
> ---
>
> Key: SQOOP-3346
> URL: https://issues.apache.org/jira/browse/SQOOP-3346
> Project: Sqoop
>  Issue Type: Sub-task
>Reporter: Boglarka Egyed
>Assignee: Boglarka Egyed
>Priority: Major
>
> Support for AWS temporary credentials has been introduced in Hadoop 2.8.0 
> based on HADOOP-12537 and it would make more sense to test and support this 
> capability too with Sqoop.
> There is [SQOOP-3305|https://reviews.apache.org/r/66300/bugs/SQOOP-3305/] 
> being open for upgrading Hadoop to 3.0.0 however it has several issues 
> described in [https://reviews.apache.org/r/66300/] currently thus I would 
> like to proceed with an "intermediate" upgrade to 2.8.0 to enable development 
> on S3 front. [~dvoros] are you OK with this?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Review Request 67971: SQOOP-3346: Upgrade Hadoop version to 2.8.0

2018-07-19 Thread Boglarka Egyed 

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

Review request for Sqoop, daniel voros, Fero Szabo, and Szabolcs Vasas.


Bugs: SQOOP-3346
https://issues.apache.org/jira/browse/SQOOP-3346


Repository: sqoop-trunk


Description
---

Upgrading Hadoop version from 2.6.0 to 2.8.0 and some related code changes.


Diffs
-

  ivy/libraries.properties 565a8bf50cdd88597a2a502d2fdbce2d5c8585ef 
  src/java/org/apache/sqoop/config/ConfigurationConstants.java 
666852c2af2f7636bd068c24e5df32173b185603 
  src/java/org/apache/sqoop/config/ConfigurationHelper.java 
fb2ab031caef023dfbd8130814d07416dbf4db14 
  src/java/org/apache/sqoop/mapreduce/JobBase.java 
6d1e04992c0e1d45a24e22fcd765c286e7414578 
  src/java/org/apache/sqoop/tool/ImportTool.java 
f7310b939a667e4434a78bdbc50f9520fe72f8a6 
  src/test/org/apache/sqoop/TestSqoopOptions.java 
ba4a4d44f36c155318092bdcc71588c476e84e2d 
  src/test/org/apache/sqoop/manager/sqlserver/SQLServerParseMethodsTest.java 
833ebe8a14e438daa7fbb2eae13dc0d04bec3bb8 
  src/test/org/apache/sqoop/orm/TestParseMethods.java 
46bb52d562991bc9c3443b8a26c7a7f9996d72d2 


Diff: https://reviews.apache.org/r/67971/diff/1/


Testing
---

Ran unit and 3rd party tests successfully.


Thanks,

Boglarka Egyed

[jira] [Commented] (SQOOP-3346) Upgrade Hadoop version to 2.8.0

2018-07-19 Thread Daniel Voros (JIRA) 


[ 
https://issues.apache.org/jira/browse/SQOOP-3346?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16549011#comment-16549011
 ] 

Daniel Voros commented on SQOOP-3346:
-

Yes, I agree with you. Don't block this until SQOOP-3305 is done!

> Upgrade Hadoop version to 2.8.0
> ---
>
> Key: SQOOP-3346
> URL: https://issues.apache.org/jira/browse/SQOOP-3346
> Project: Sqoop
>  Issue Type: Sub-task
>Reporter: Boglarka Egyed
>Assignee: Boglarka Egyed
>Priority: Major
>
> Support for AWS temporary credentials has been introduced in Hadoop 2.8.0 
> based on HADOOP-12537 and it would make more sense to test and support this 
> capability too with Sqoop.
> There is [SQOOP-3305|https://reviews.apache.org/r/66300/bugs/SQOOP-3305/] 
> being open for upgrading Hadoop to 3.0.0 however it has several issues 
> described in [https://reviews.apache.org/r/66300/] currently thus I would 
> like to proceed with an "intermediate" upgrade to 2.8.0 to enable development 
> on S3 front. [~dvoros] are you OK with this?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (SQOOP-3346) Upgrade Hadoop version to 2.8.0

2018-07-19 Thread Boglarka Egyed (JIRA) 
Boglarka Egyed created SQOOP-3346:
-

 Summary: Upgrade Hadoop version to 2.8.0
 Key: SQOOP-3346
 URL: https://issues.apache.org/jira/browse/SQOOP-3346
 Project: Sqoop
  Issue Type: Sub-task
Reporter: Boglarka Egyed
Assignee: Boglarka Egyed


Support for AWS temporary credentials has been introduced in Hadoop 2.8.0 based 
on HADOOP-12537 and it would make more sense to test and support this 
capability too with Sqoop.

There is [SQOOP-3305|https://reviews.apache.org/r/66300/bugs/SQOOP-3305/] being 
open for upgrading Hadoop to 3.0.0 however it has several issues described in 
[https://reviews.apache.org/r/66300/] currently thus I would like to proceed 
with an "intermediate" upgrade to 2.8.0 to enable development on S3 front. 
[~dvoros] are you OK with this?



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

[jira] [Created] (SQOOP-3345) Test and document current S3 capabilities

2018-07-19 Thread Boglarka Egyed (JIRA) 
Boglarka Egyed created SQOOP-3345:
-

 Summary: Test and document current S3 capabilities
 Key: SQOOP-3345
 URL: https://issues.apache.org/jira/browse/SQOOP-3345
 Project: Sqoop
  Issue Type: Bug
Affects Versions: 1.4.7
Reporter: Boglarka Egyed
Assignee: Boglarka Egyed


The 
{{[hadoop-aws|https://hadoop.apache.org/docs/stable/hadoop-aws/tools/hadoop-aws/index.html]}}
 module provides support for AWS integration which could be used by Sqoop to 
work with S3.

The scope of this task is to explore and document the capabilities of the 
current Sqoop implementation regarding the "from RDBMS to S3" use case by 
adding the {{hadoop-aws}} dependency.

 

This is an umbrella ticket to collect all the related tasks and also the first 
step to enable further improvements to support all the required use cases.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)