subject:"Re\: Review Request 66361\: Implement HiveServer2 client"

Re: Review Request 66361: Implement HiveServer2 client

2018-04-18 Thread Fero Szabo via Review Board


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


Ship it!




Ship It!

- Fero Szabo


On April 17, 2018, 10:14 a.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 17, 2018, 10:14 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> 
> 
> Diffs
> -
> 
>   build.xml 7f68b573c65a61150ca78d158084586c87775d84 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
>   src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactoryInitializer.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 
> b7a25b7809e0d50166966a77161dc8ff603fb2d2 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> b02e4fe7fda25c7f8171c7db17d15a7987459687 
>   src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
> d259566180369a55d490144e6f865e728f4f2e61 
>   src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
> 18f7a0af48d972d5186e9414475e080f1eb765f3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> e9920058858653bec7407bf7992eb6445401e813 
>   
> src/test/org/apache/sqoop/hive/HiveServer2ConnectionFactoryInitializerTest.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
> 8bdc3beb3677312ec0ee2e612616358bca4ca838 
>   src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestImportTool.java 
> 1c0cf4d863692f75bb8831e834fae47fc18b5df5 
> 
> 
> Diff: https://reviews.apache.org/r/66361/diff/6/
> 
> 
> Testing
> ---
> 
> Ran unit and third party tests suite.
> 
> 
> Thanks,
> 
> Szabolcs Vasas
> 
>

Re: Review Request 66361: Implement HiveServer2 client

2018-04-17 Thread Szabolcs Vasas


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

(Updated April 17, 2018, 10:14 a.m.)


Review request for Sqoop.


Changes
---

HiveClientFactory is refactored a bit.


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


Repository: sqoop-trunk


Description
---

This JIRA covers the implementation of the client for HiveServer2 and its 
integration into the classes which use HiveImport.

- HiveClient interface is introduced with 2 implementation:
  - HiveImport: this is the original implementation which uses HiveCLI
  - HiveServer2Client: the new clients which connects to HS2 using JDBC 
connection
  - The common code is extracted to HiveCommon class
- HiveClient should be instantiated using HiveClientFactory which creates and 
configures the right HiveClient based on the configuration in SqoopOptions
- HiveMiniCluster is introduced with a couple of helper classes to enable 
end-to-end HS2 tests
- A couple of new options are added to SqoopOptions to be able to configure the 
connection to HS2
- Validation is implemented for these new options


Diffs (updated)
-

  build.xml 7f68b573c65a61150ca78d158084586c87775d84 
  ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
  src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
  src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
  src/java/org/apache/sqoop/SqoopOptions.java 
651cebd69ee7e75d06c75945e3607c4fab7eb11c 
  src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveImport.java 
c2729119d31f7e585f204f2d31b2051eea71b72b 
  src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactoryInitializer.java 
PRE-CREATION 
  src/java/org/apache/sqoop/hive/TableDefWriter.java 
b7a25b7809e0d50166966a77161dc8ff603fb2d2 
  src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
b02e4fe7fda25c7f8171c7db17d15a7987459687 
  src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
d259566180369a55d490144e6f865e728f4f2e61 
  src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
18f7a0af48d972d5186e9414475e080f1eb765f3 
  src/java/org/apache/sqoop/tool/ImportTool.java 
e9920058858653bec7407bf7992eb6445401e813 
  
src/test/org/apache/sqoop/hive/HiveServer2ConnectionFactoryInitializerTest.java 
PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
8bdc3beb3677312ec0ee2e612616358bca4ca838 
  src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
PRE-CREATION 
  src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java PRE-CREATION 
  
src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
 PRE-CREATION 
  src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
PRE-CREATION 
  
src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
 PRE-CREATION 
  src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
  src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
PRE-CREATION 
  src/test/org/apache/sqoop/tool/TestImportTool.java 
1c0cf4d863692f75bb8831e834fae47fc18b5df5 


Diff: https://reviews.apache.org/r/66361/diff/6/

Changes: https://reviews.apache.org/r/66361/diff/5-6/


Testing
---

Ran unit and third party tests suite.


Thanks,

Szabolcs Vasas

Re: Review Request 66361: Implement HiveServer2 client

2018-04-17 Thread Szabolcs Vasas



> On April 16, 2018, 12:51 p.m., Fero Szabo wrote:
> > src/java/org/apache/sqoop/hive/HiveClientFactory.java
> > Lines 54 (patched)
> > 
> >
> > I think you mentioned that this is package-private visibility so it can 
> > be used in testing. 
> > 
> > You could consider using the 
> > org.assertj.core.util.VisibleForTesting
> > annotation as well, though I'm not sure if that's actually better or 
> > worse.

This part of the code is refactored now.


- Szabolcs


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


On April 17, 2018, 10:14 a.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 17, 2018, 10:14 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> 
> 
> Diffs
> -
> 
>   build.xml 7f68b573c65a61150ca78d158084586c87775d84 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
>   src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactoryInitializer.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 
> b7a25b7809e0d50166966a77161dc8ff603fb2d2 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> b02e4fe7fda25c7f8171c7db17d15a7987459687 
>   src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
> d259566180369a55d490144e6f865e728f4f2e61 
>   src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
> 18f7a0af48d972d5186e9414475e080f1eb765f3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> e9920058858653bec7407bf7992eb6445401e813 
>   
> src/test/org/apache/sqoop/hive/HiveServer2ConnectionFactoryInitializerTest.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
> 8bdc3beb3677312ec0ee2e612616358bca4ca838 
>   src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestImportTool.java 
> 1c0cf4d863692f75bb8831e834fae47fc18b5df5 
> 
> 
> Diff: https://reviews.apache.org/r/66361/diff/6/
> 
> 
> Testing
> ---
> 
> Ran unit and third party tests suite.
> 
> 
> Thanks,
> 
> Szabolcs

Re: Review Request 66361: Implement HiveServer2 client

2018-04-17 Thread Szabolcs Vasas



> On April 16, 2018, 12:51 p.m., Fero Szabo wrote:
> > src/java/org/apache/sqoop/hive/HiveServer2Client.java
> > Lines 88 (patched)
> > 
> >
> > Why not put this into a finally statement? No need to clean up after 
> > error?

The original implementation worked the same way so I did not want to change it, 
but the idea here is to not delete the files imported into HDFS when the Hive 
CREATE TABLE or the LOAD DATA INPATH command fails, so the user could manually 
load the data into Hive.


> On April 16, 2018, 12:51 p.m., Fero Szabo wrote:
> > src/java/org/apache/sqoop/tool/BaseSqoopTool.java
> > Lines 136 (patched)
> > 
> >
> > I'm fine with abbreviations, however these might confuse those reading 
> > the code in the long run. It would make it easier for newbies to onboard 
> > and help less profficient users to understand what a command means if we 
> > weren't using them.
> > 
> > So, we might want to follow the convention of not using abbreviations 
> > in the names of fields, constants, methods, etc.
> > 
> > What is your opinion? This might be a good quesiton for the community 
> > as well, as we don't really have coding guidelines.
> 
> Boglarka Egyed wrote:
> I think this is a really good point however even the documentation uses 
> HS2 abbreviation thus it may not be that misleading as it seems to be: 
> https://cwiki.apache.org/confluence/display/Hive/HiveServer2+Overview

Yes, usually I like to avoid abbreviations but I think in this case HS2 is a 
commonly known one and having --hiveserver2-url, --hiveserver2-user etc. as 
parameter names might make the Sqoop too long so I would keep it as is.


> On April 16, 2018, 12:51 p.m., Fero Szabo wrote:
> > src/java/org/apache/sqoop/tool/BaseSqoopTool.java
> > Line 1854 (original), 1886-1904 (patched)
> > 
> >
> > Nice validation!
> > 
> > You could consider extracting two helper methods here, I believe: 
> > - validateAllOptionsAreSet(SqoopOption, List optionkeys) throws 
> > InvalidOptionsException
> > - validateOnlyOneisSet(SqoopOption, List optionkeys) throws 
> > InvalidOptionsException
> > 
> > or something similar...
> > 
> > It would help validators reuse your nice error messages in the future!

Yes, good idea but the problem is that the SqoopOptions fields cannot be easily 
found by the option key so these methods would be quite complex. I would keep 
this method as is for now.


- Szabolcs


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


On April 16, 2018, 9:12 a.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 16, 2018, 9:12 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> 
> 
> Diffs
> -
> 
>   build.xml 7f68b573c65a61150ca78d158084586c87775d84 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
>   src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>

Re: Review Request 66361: Implement HiveServer2 client

2018-04-16 Thread Boglarka Egyed


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


Ship it!




Thanks for fixing my findigs! I left one comment regarding the hiveserver2 vs. 
hs2 naming question but in general I'm OK with using hs2 too.

- Boglarka Egyed


On April 16, 2018, 9:12 a.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 16, 2018, 9:12 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> 
> 
> Diffs
> -
> 
>   build.xml 7f68b573c65a61150ca78d158084586c87775d84 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
>   src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 
> b7a25b7809e0d50166966a77161dc8ff603fb2d2 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> b02e4fe7fda25c7f8171c7db17d15a7987459687 
>   src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
> d259566180369a55d490144e6f865e728f4f2e61 
>   src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
> 18f7a0af48d972d5186e9414475e080f1eb765f3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> e9920058858653bec7407bf7992eb6445401e813 
>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
> 8bdc3beb3677312ec0ee2e612616358bca4ca838 
>   src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestImportTool.java 
> 1c0cf4d863692f75bb8831e834fae47fc18b5df5 
> 
> 
> Diff: https://reviews.apache.org/r/66361/diff/5/
> 
> 
> Testing
> ---
> 
> Ran unit and third party tests suite.
> 
> 
> Thanks,
> 
> Szabolcs Vasas
> 
>

Re: Review Request 66361: Implement HiveServer2 client

2018-04-16 Thread Boglarka Egyed



> On April 16, 2018, 12:51 p.m., Fero Szabo wrote:
> > src/java/org/apache/sqoop/tool/BaseSqoopTool.java
> > Lines 136 (patched)
> > 
> >
> > I'm fine with abbreviations, however these might confuse those reading 
> > the code in the long run. It would make it easier for newbies to onboard 
> > and help less profficient users to understand what a command means if we 
> > weren't using them.
> > 
> > So, we might want to follow the convention of not using abbreviations 
> > in the names of fields, constants, methods, etc.
> > 
> > What is your opinion? This might be a good quesiton for the community 
> > as well, as we don't really have coding guidelines.

I think this is a really good point however even the documentation uses HS2 
abbreviation thus it may not be that misleading as it seems to be: 
https://cwiki.apache.org/confluence/display/Hive/HiveServer2+Overview


- Boglarka


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


On April 16, 2018, 9:12 a.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 16, 2018, 9:12 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> 
> 
> Diffs
> -
> 
>   build.xml 7f68b573c65a61150ca78d158084586c87775d84 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
>   src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 
> b7a25b7809e0d50166966a77161dc8ff603fb2d2 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> b02e4fe7fda25c7f8171c7db17d15a7987459687 
>   src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
> d259566180369a55d490144e6f865e728f4f2e61 
>   src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
> 18f7a0af48d972d5186e9414475e080f1eb765f3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> e9920058858653bec7407bf7992eb6445401e813 
>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
> 8bdc3beb3677312ec0ee2e612616358bca4ca838 
>   src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
> PRE-CREATION 
>

Re: Review Request 66361: Implement HiveServer2 client

2018-04-16 Thread daniel voros


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


Ship it!




Ship It!

- daniel voros


On April 16, 2018, 9:12 a.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 16, 2018, 9:12 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> 
> 
> Diffs
> -
> 
>   build.xml 7f68b573c65a61150ca78d158084586c87775d84 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
>   src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 
> b7a25b7809e0d50166966a77161dc8ff603fb2d2 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> b02e4fe7fda25c7f8171c7db17d15a7987459687 
>   src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
> d259566180369a55d490144e6f865e728f4f2e61 
>   src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
> 18f7a0af48d972d5186e9414475e080f1eb765f3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> e9920058858653bec7407bf7992eb6445401e813 
>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
> 8bdc3beb3677312ec0ee2e612616358bca4ca838 
>   src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestImportTool.java 
> 1c0cf4d863692f75bb8831e834fae47fc18b5df5 
> 
> 
> Diff: https://reviews.apache.org/r/66361/diff/5/
> 
> 
> Testing
> ---
> 
> Ran unit and third party tests suite.
> 
> 
> Thanks,
> 
> Szabolcs Vasas
> 
>

Re: Review Request 66361: Implement HiveServer2 client

2018-04-16 Thread Fero Szabo via Review Board


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




src/java/org/apache/sqoop/hive/HiveClientFactory.java
Lines 54 (patched)


I think you mentioned that this is package-private visibility so it can be 
used in testing. 

You could consider using the 
org.assertj.core.util.VisibleForTesting
annotation as well, though I'm not sure if that's actually better or worse.



src/java/org/apache/sqoop/hive/HiveServer2Client.java
Lines 88 (patched)


Why not put this into a finally statement? No need to clean up after error?



src/java/org/apache/sqoop/tool/BaseSqoopTool.java
Lines 136 (patched)


I'm fine with abbreviations, however these might confuse those reading the 
code in the long run. It would make it easier for newbies to onboard and help 
less profficient users to understand what a command means if we weren't using 
them.

So, we might want to follow the convention of not using abbreviations in 
the names of fields, constants, methods, etc.

What is your opinion? This might be a good quesiton for the community as 
well, as we don't really have coding guidelines.



src/java/org/apache/sqoop/tool/BaseSqoopTool.java
Line 1854 (original), 1886-1904 (patched)


Nice validation!

You could consider extracting two helper methods here, I believe: 
- validateAllOptionsAreSet(SqoopOption, List optionkeys) throws 
InvalidOptionsException
- validateOnlyOneisSet(SqoopOption, List optionkeys) throws 
InvalidOptionsException

or something similar...

It would help validators reuse your nice error messages in the future!


- Fero Szabo


On April 16, 2018, 9:12 a.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 16, 2018, 9:12 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> 
> 
> Diffs
> -
> 
>   build.xml 7f68b573c65a61150ca78d158084586c87775d84 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
>   src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 
> b7a25b7809e0d50166966a77161dc8ff603fb2d2 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> b02e4fe7fda25c7f8171c7db17d15a7987459687 
>   src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
> d259566180369a55d490144e6f865e728f4f2e61 
>   src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
> 18f7a0af48d972d5186e9414475e080f1eb765f3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> e9920058858653bec7407bf7992eb6445401e813 
>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
>

Re: Review Request 66361: Implement HiveServer2 client

2018-04-16 Thread Szabolcs Vasas



> On April 6, 2018, 3:49 p.m., Fero Szabo wrote:
> > src/java/org/apache/sqoop/hive/HiveServer2Client.java
> > Lines 75 (patched)
> > 
> >
> > Is the package private visibility intentional?
> > 
> > Also applies for the remaining functions.
> 
> Szabolcs Vasas wrote:
> These are visible for testing, but I am considering refactoring this part 
> and extract these methods to another class.

I have decided to leave this code as it is for now, so resolving this comment.


- Szabolcs


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


On April 16, 2018, 9:12 a.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 16, 2018, 9:12 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> 
> 
> Diffs
> -
> 
>   build.xml 7f68b573c65a61150ca78d158084586c87775d84 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
>   src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 
> b7a25b7809e0d50166966a77161dc8ff603fb2d2 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> b02e4fe7fda25c7f8171c7db17d15a7987459687 
>   src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
> d259566180369a55d490144e6f865e728f4f2e61 
>   src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
> 18f7a0af48d972d5186e9414475e080f1eb765f3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> e9920058858653bec7407bf7992eb6445401e813 
>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
> 8bdc3beb3677312ec0ee2e612616358bca4ca838 
>   src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestImportTool.java 
> 1c0cf4d863692f75bb8831e834fae47fc18b5df5 
> 
> 
> Diff: https://reviews.apache.org/r/66361/diff/5/
> 
> 
> Testing
> ---
> 
> Ran unit and third party tests suite.
> 
> 
> Thanks,
> 
> Szabolcs Vasas
> 
>

Re: Review Request 66361: Implement HiveServer2 client

2018-04-16 Thread Szabolcs Vasas



> On April 13, 2018, 2:33 p.m., daniel voros wrote:
> > Hey Szabolcs,
> > 
> > I'm trying to run the latest patch on a (non-kerberized) cluster, but I get 
> > the following:
> > 
> > ```
> > 18/04/13 13:50:47 INFO hive.HiveServer2ConnectionFactory: Creating 
> > connection to HiveServer2 as: hdfs (auth:SIMPLE)
> > 18/04/13 13:50:47 INFO jdbc.Utils: Supplied authorities: hostname:1
> > 18/04/13 13:50:47 INFO jdbc.Utils: Resolved authority: hostname:1
> > 18/04/13 13:50:48 ERROR sqoop.Sqoop: Got exception running Sqoop: 
> > java.lang.RuntimeException: Error executing Hive import.
> > java.lang.RuntimeException: Error executing Hive import.
> > at 
> > org.apache.sqoop.hive.HiveServer2Client.executeHiveImport(HiveServer2Client.java:85)
> > at 
> > org.apache.sqoop.hive.HiveServer2Client.importTable(HiveServer2Client.java:63)
> > at org.apache.sqoop.tool.ImportTool.importTable(ImportTool.java:547)
> > at org.apache.sqoop.tool.ImportTool.run(ImportTool.java:632)
> > at org.apache.sqoop.Sqoop.run(Sqoop.java:145)
> > at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:76)
> > at org.apache.sqoop.Sqoop.runSqoop(Sqoop.java:181)
> > at org.apache.sqoop.Sqoop.runTool(Sqoop.java:232)
> > at org.apache.sqoop.Sqoop.runTool(Sqoop.java:241)
> > at org.apache.sqoop.Sqoop.main(Sqoop.java:250)
> > 
> > ...
> > 
> > 
> > Caused by: org.apache.hadoop.hive.ql.parse.SemanticException: Line 1:17 
> > Invalid path ''hdfs://hostname:8020/user/hdfs/asd''
> > at 
> > org.apache.hadoop.hive.ql.parse.LoadSemanticAnalyzer.applyConstraintsAndGetFiles(LoadSemanticAnalyzer.java:160)
> > at 
> > org.apache.hadoop.hive.ql.parse.LoadSemanticAnalyzer.analyzeInternal(LoadSemanticAnalyzer.java:225)
> > at 
> > org.apache.hadoop.hive.ql.parse.BaseSemanticAnalyzer.analyze(BaseSemanticAnalyzer.java:238)
> > at org.apache.hadoop.hive.ql.Driver.compile(Driver.java:465)
> > at org.apache.hadoop.hive.ql.Driver.compile(Driver.java:321)
> > at 
> > org.apache.hadoop.hive.ql.Driver.compileInternal(Driver.java:1224)
> > at 
> > org.apache.hadoop.hive.ql.Driver.compileAndRespond(Driver.java:1218)
> > at 
> > org.apache.hive.service.cli.operation.SQLOperation.prepare(SQLOperation.java:146)
> > ... 26 more
> > Caused by: org.apache.hadoop.security.AccessControlException: Permission 
> > denied: user=anonymous, access=EXECUTE, 
> > inode="/user/hdfs/asd":hdfs:hdfs:drwx--
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.check(FSPermissionChecker.java:353)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.checkTraverse(FSPermissionChecker.java:292)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.checkPermission(FSPermissionChecker.java:238)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.checkPermission(FSPermissionChecker.java:190)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSDirectory.checkPermission(FSDirectory.java:1950)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSDirStatAndListingOp.getFileInfo(FSDirStatAndListingOp.java:108)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSNamesystem.getFileInfo(FSNamesystem.java:4142)
> > ...
> > ```
> > 
> > Note that according to the log message, we're running as 'hdfs' user 
> > (current OS user), but HDFS checks permission for anonymous.
> > 
> > Could it be the result of 
> > org/apache/sqoop/hive/HiveServer2ConnectionFactory.java:42 (passing 
> > username=null)?
> > ```
> >   public HiveServer2ConnectionFactory(String connectionString) {
> > this(connectionString, null, null);
> >   }
> > ```
> > 
> > Also, it might make sense to use the --hs2-user parameter in the 
> > non-kerberized case as well. Like beeline allows you to override user with 
> > `-n username`. What do you think?
> > 
> > Regards,
> > Daniel
> 
> Szabolcs Vasas wrote:
> Hi Dani,
> 
> Thank you for checking this!
> I was aiming for the kerberos authentication only so I have never tested 
> it in non-kerberized environment but it totally makes sense to fix this 
> scenario. However in that case we should add a --hs2-password parameter as 
> well, shouldn't we? Or does Hive accept connections without password 
> specified?
> 
> daniel voros wrote:
> Paswordless connections are also allowed. However, since beeline has a 
> `-p `, I think it would make sense to add `--hs2-password`. Thank 
> you!

I have added the fix to support password-less connections on non-secure 
clusters but I think implementing a secure way to provide the password for HS2 
is a bit more complex so I would cover it in another JIRA since this one is 
already big enough.


- Szabolcs


---
This is an automatically generated e-mail. To reply, visit:

Re: Review Request 66361: Implement HiveServer2 client

2018-04-16 Thread Szabolcs Vasas


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

(Updated April 16, 2018, 9:12 a.m.)


Review request for Sqoop.


Changes
---

Password-less authentication on non-secure clusters is supported now.


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


Repository: sqoop-trunk


Description
---

This JIRA covers the implementation of the client for HiveServer2 and its 
integration into the classes which use HiveImport.

- HiveClient interface is introduced with 2 implementation:
  - HiveImport: this is the original implementation which uses HiveCLI
  - HiveServer2Client: the new clients which connects to HS2 using JDBC 
connection
  - The common code is extracted to HiveCommon class
- HiveClient should be instantiated using HiveClientFactory which creates and 
configures the right HiveClient based on the configuration in SqoopOptions
- HiveMiniCluster is introduced with a couple of helper classes to enable 
end-to-end HS2 tests
- A couple of new options are added to SqoopOptions to be able to configure the 
connection to HS2
- Validation is implemented for these new options


Diffs (updated)
-

  build.xml 7f68b573c65a61150ca78d158084586c87775d84 
  ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
  src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
  src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
  src/java/org/apache/sqoop/SqoopOptions.java 
651cebd69ee7e75d06c75945e3607c4fab7eb11c 
  src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveImport.java 
c2729119d31f7e585f204f2d31b2051eea71b72b 
  src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/TableDefWriter.java 
b7a25b7809e0d50166966a77161dc8ff603fb2d2 
  src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
b02e4fe7fda25c7f8171c7db17d15a7987459687 
  src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
d259566180369a55d490144e6f865e728f4f2e61 
  src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
18f7a0af48d972d5186e9414475e080f1eb765f3 
  src/java/org/apache/sqoop/tool/ImportTool.java 
e9920058858653bec7407bf7992eb6445401e813 
  src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
8bdc3beb3677312ec0ee2e612616358bca4ca838 
  src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
PRE-CREATION 
  src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java PRE-CREATION 
  
src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
 PRE-CREATION 
  src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
PRE-CREATION 
  
src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
 PRE-CREATION 
  src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
  src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
PRE-CREATION 
  src/test/org/apache/sqoop/tool/TestImportTool.java 
1c0cf4d863692f75bb8831e834fae47fc18b5df5 


Diff: https://reviews.apache.org/r/66361/diff/5/

Changes: https://reviews.apache.org/r/66361/diff/4-5/


Testing
---

Ran unit and third party tests suite.


Thanks,

Szabolcs Vasas

Re: Review Request 66361: Implement HiveServer2 client

2018-04-13 Thread Szabolcs Vasas



> On April 13, 2018, 2:33 p.m., daniel voros wrote:
> > Hey Szabolcs,
> > 
> > I'm trying to run the latest patch on a (non-kerberized) cluster, but I get 
> > the following:
> > 
> > ```
> > 18/04/13 13:50:47 INFO hive.HiveServer2ConnectionFactory: Creating 
> > connection to HiveServer2 as: hdfs (auth:SIMPLE)
> > 18/04/13 13:50:47 INFO jdbc.Utils: Supplied authorities: hostname:1
> > 18/04/13 13:50:47 INFO jdbc.Utils: Resolved authority: hostname:1
> > 18/04/13 13:50:48 ERROR sqoop.Sqoop: Got exception running Sqoop: 
> > java.lang.RuntimeException: Error executing Hive import.
> > java.lang.RuntimeException: Error executing Hive import.
> > at 
> > org.apache.sqoop.hive.HiveServer2Client.executeHiveImport(HiveServer2Client.java:85)
> > at 
> > org.apache.sqoop.hive.HiveServer2Client.importTable(HiveServer2Client.java:63)
> > at org.apache.sqoop.tool.ImportTool.importTable(ImportTool.java:547)
> > at org.apache.sqoop.tool.ImportTool.run(ImportTool.java:632)
> > at org.apache.sqoop.Sqoop.run(Sqoop.java:145)
> > at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:76)
> > at org.apache.sqoop.Sqoop.runSqoop(Sqoop.java:181)
> > at org.apache.sqoop.Sqoop.runTool(Sqoop.java:232)
> > at org.apache.sqoop.Sqoop.runTool(Sqoop.java:241)
> > at org.apache.sqoop.Sqoop.main(Sqoop.java:250)
> > 
> > ...
> > 
> > 
> > Caused by: org.apache.hadoop.hive.ql.parse.SemanticException: Line 1:17 
> > Invalid path ''hdfs://hostname:8020/user/hdfs/asd''
> > at 
> > org.apache.hadoop.hive.ql.parse.LoadSemanticAnalyzer.applyConstraintsAndGetFiles(LoadSemanticAnalyzer.java:160)
> > at 
> > org.apache.hadoop.hive.ql.parse.LoadSemanticAnalyzer.analyzeInternal(LoadSemanticAnalyzer.java:225)
> > at 
> > org.apache.hadoop.hive.ql.parse.BaseSemanticAnalyzer.analyze(BaseSemanticAnalyzer.java:238)
> > at org.apache.hadoop.hive.ql.Driver.compile(Driver.java:465)
> > at org.apache.hadoop.hive.ql.Driver.compile(Driver.java:321)
> > at 
> > org.apache.hadoop.hive.ql.Driver.compileInternal(Driver.java:1224)
> > at 
> > org.apache.hadoop.hive.ql.Driver.compileAndRespond(Driver.java:1218)
> > at 
> > org.apache.hive.service.cli.operation.SQLOperation.prepare(SQLOperation.java:146)
> > ... 26 more
> > Caused by: org.apache.hadoop.security.AccessControlException: Permission 
> > denied: user=anonymous, access=EXECUTE, 
> > inode="/user/hdfs/asd":hdfs:hdfs:drwx--
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.check(FSPermissionChecker.java:353)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.checkTraverse(FSPermissionChecker.java:292)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.checkPermission(FSPermissionChecker.java:238)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.checkPermission(FSPermissionChecker.java:190)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSDirectory.checkPermission(FSDirectory.java:1950)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSDirStatAndListingOp.getFileInfo(FSDirStatAndListingOp.java:108)
> > at 
> > org.apache.hadoop.hdfs.server.namenode.FSNamesystem.getFileInfo(FSNamesystem.java:4142)
> > ...
> > ```
> > 
> > Note that according to the log message, we're running as 'hdfs' user 
> > (current OS user), but HDFS checks permission for anonymous.
> > 
> > Could it be the result of 
> > org/apache/sqoop/hive/HiveServer2ConnectionFactory.java:42 (passing 
> > username=null)?
> > ```
> >   public HiveServer2ConnectionFactory(String connectionString) {
> > this(connectionString, null, null);
> >   }
> > ```
> > 
> > Also, it might make sense to use the --hs2-user parameter in the 
> > non-kerberized case as well. Like beeline allows you to override user with 
> > `-n username`. What do you think?
> > 
> > Regards,
> > Daniel

Hi Dani,

Thank you for checking this!
I was aiming for the kerberos authentication only so I have never tested it in 
non-kerberized environment but it totally makes sense to fix this scenario. 
However in that case we should add a --hs2-password parameter as well, 
shouldn't we? Or does Hive accept connections without password specified?


- Szabolcs


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


On April 12, 2018, 2:10 p.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 12, 2018, 2:10 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
>

Re: Review Request 66361: Implement HiveServer2 client

2018-04-13 Thread daniel voros

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

Hey Szabolcs,

I'm trying to run the latest patch on a (non-kerberized) cluster, but I get the 
following:

```
18/04/13 13:50:47 INFO hive.HiveServer2ConnectionFactory: Creating connection 
to HiveServer2 as: hdfs (auth:SIMPLE)
18/04/13 13:50:47 INFO jdbc.Utils: Supplied authorities: hostname:1
18/04/13 13:50:47 INFO jdbc.Utils: Resolved authority: hostname:1
18/04/13 13:50:48 ERROR sqoop.Sqoop: Got exception running Sqoop: 
java.lang.RuntimeException: Error executing Hive import.
java.lang.RuntimeException: Error executing Hive import.
at 
org.apache.sqoop.hive.HiveServer2Client.executeHiveImport(HiveServer2Client.java:85)
at 
org.apache.sqoop.hive.HiveServer2Client.importTable(HiveServer2Client.java:63)
at org.apache.sqoop.tool.ImportTool.importTable(ImportTool.java:547)
at org.apache.sqoop.tool.ImportTool.run(ImportTool.java:632)
at org.apache.sqoop.Sqoop.run(Sqoop.java:145)
at org.apache.hadoop.util.ToolRunner.run(ToolRunner.java:76)
at org.apache.sqoop.Sqoop.runSqoop(Sqoop.java:181)
at org.apache.sqoop.Sqoop.runTool(Sqoop.java:232)
at org.apache.sqoop.Sqoop.runTool(Sqoop.java:241)
at org.apache.sqoop.Sqoop.main(Sqoop.java:250)

...

Caused by: org.apache.hadoop.hive.ql.parse.SemanticException: Line 1:17 Invalid 
path ''hdfs://hostname:8020/user/hdfs/asd''
at 
org.apache.hadoop.hive.ql.parse.LoadSemanticAnalyzer.applyConstraintsAndGetFiles(LoadSemanticAnalyzer.java:160)
at 
org.apache.hadoop.hive.ql.parse.LoadSemanticAnalyzer.analyzeInternal(LoadSemanticAnalyzer.java:225)
at 
org.apache.hadoop.hive.ql.parse.BaseSemanticAnalyzer.analyze(BaseSemanticAnalyzer.java:238)
at org.apache.hadoop.hive.ql.Driver.compile(Driver.java:465)
at org.apache.hadoop.hive.ql.Driver.compile(Driver.java:321)
at org.apache.hadoop.hive.ql.Driver.compileInternal(Driver.java:1224)
at org.apache.hadoop.hive.ql.Driver.compileAndRespond(Driver.java:1218)
at 
org.apache.hive.service.cli.operation.SQLOperation.prepare(SQLOperation.java:146)
... 26 more
Caused by: org.apache.hadoop.security.AccessControlException: Permission 
denied: user=anonymous, access=EXECUTE, 
inode="/user/hdfs/asd":hdfs:hdfs:drwx--
at 
org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.check(FSPermissionChecker.java:353)
at 
org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.checkTraverse(FSPermissionChecker.java:292)
at 
org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.checkPermission(FSPermissionChecker.java:238)
at 
org.apache.hadoop.hdfs.server.namenode.FSPermissionChecker.checkPermission(FSPermissionChecker.java:190)
at 
org.apache.hadoop.hdfs.server.namenode.FSDirectory.checkPermission(FSDirectory.java:1950)
at 
org.apache.hadoop.hdfs.server.namenode.FSDirStatAndListingOp.getFileInfo(FSDirStatAndListingOp.java:108)
at 
org.apache.hadoop.hdfs.server.namenode.FSNamesystem.getFileInfo(FSNamesystem.java:4142)
...
```

Note that according to the log message, we're running as 'hdfs' user (current 
OS user), but HDFS checks permission for anonymous.

Could it be the result of 
org/apache/sqoop/hive/HiveServer2ConnectionFactory.java:42 (passing 
username=null)?
```
  public HiveServer2ConnectionFactory(String connectionString) {
this(connectionString, null, null);
  }
```

Also, it might make sense to use the --hs2-user parameter in the non-kerberized 
case as well. Like beeline allows you to override user with `-n username`. What 
do you think?

Regards,
Daniel

- daniel voros

On April 12, 2018, 2:10 p.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 12, 2018, 2:10 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper

Re: Review Request 66361: Implement HiveServer2 client

2018-04-12 Thread Szabolcs Vasas


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

(Updated April 12, 2018, 2:10 p.m.)


Review request for Sqoop.


Changes
---

Fail fast is introduced for --hs2-url and --as-parquetfile option.


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


Repository: sqoop-trunk


Description
---

This JIRA covers the implementation of the client for HiveServer2 and its 
integration into the classes which use HiveImport.

- HiveClient interface is introduced with 2 implementation:
  - HiveImport: this is the original implementation which uses HiveCLI
  - HiveServer2Client: the new clients which connects to HS2 using JDBC 
connection
  - The common code is extracted to HiveCommon class
- HiveClient should be instantiated using HiveClientFactory which creates and 
configures the right HiveClient based on the configuration in SqoopOptions
- HiveMiniCluster is introduced with a couple of helper classes to enable 
end-to-end HS2 tests
- A couple of new options are added to SqoopOptions to be able to configure the 
connection to HS2
- Validation is implemented for these new options


Diffs (updated)
-

  build.xml 7f68b573c65a61150ca78d158084586c87775d84 
  ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
  src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
  src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
  src/java/org/apache/sqoop/SqoopOptions.java 
651cebd69ee7e75d06c75945e3607c4fab7eb11c 
  src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveImport.java 
c2729119d31f7e585f204f2d31b2051eea71b72b 
  src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/TableDefWriter.java 
b7a25b7809e0d50166966a77161dc8ff603fb2d2 
  src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
b02e4fe7fda25c7f8171c7db17d15a7987459687 
  src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
d259566180369a55d490144e6f865e728f4f2e61 
  src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
18f7a0af48d972d5186e9414475e080f1eb765f3 
  src/java/org/apache/sqoop/tool/ImportTool.java 
e9920058858653bec7407bf7992eb6445401e813 
  src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
8bdc3beb3677312ec0ee2e612616358bca4ca838 
  src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
PRE-CREATION 
  src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java PRE-CREATION 
  
src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
 PRE-CREATION 
  src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
PRE-CREATION 
  
src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
 PRE-CREATION 
  src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
  src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
PRE-CREATION 
  src/test/org/apache/sqoop/tool/TestImportTool.java 
1c0cf4d863692f75bb8831e834fae47fc18b5df5 


Diff: https://reviews.apache.org/r/66361/diff/4/

Changes: https://reviews.apache.org/r/66361/diff/3-4/


Testing
---

Ran unit and third party tests suite.


Thanks,

Szabolcs Vasas

Re: Review Request 66361: Implement HiveServer2 client

2018-04-12 Thread Szabolcs Vasas


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

(Updated April 12, 2018, 1:51 p.m.)


Review request for Sqoop.


Changes
---

Documentation is added.


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


Repository: sqoop-trunk


Description (updated)
---

This JIRA covers the implementation of the client for HiveServer2 and its 
integration into the classes which use HiveImport.

- HiveClient interface is introduced with 2 implementation:
  - HiveImport: this is the original implementation which uses HiveCLI
  - HiveServer2Client: the new clients which connects to HS2 using JDBC 
connection
  - The common code is extracted to HiveCommon class
- HiveClient should be instantiated using HiveClientFactory which creates and 
configures the right HiveClient based on the configuration in SqoopOptions
- HiveMiniCluster is introduced with a couple of helper classes to enable 
end-to-end HS2 tests
- A couple of new options are added to SqoopOptions to be able to configure the 
connection to HS2
- Validation is implemented for these new options


Diffs (updated)
-

  build.xml 7f68b573c65a61150ca78d158084586c87775d84 
  ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
  src/docs/user/hive-args.txt 441f54e8e0cee63595937f4e1811abc2d89f9237 
  src/docs/user/hive.txt 3dc8bb463d602d525fe5f2d07d52cb97efcbab7e 
  src/java/org/apache/sqoop/SqoopOptions.java 
651cebd69ee7e75d06c75945e3607c4fab7eb11c 
  src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveImport.java 
c2729119d31f7e585f204f2d31b2051eea71b72b 
  src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/TableDefWriter.java 
b7a25b7809e0d50166966a77161dc8ff603fb2d2 
  src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
b02e4fe7fda25c7f8171c7db17d15a7987459687 
  src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
d259566180369a55d490144e6f865e728f4f2e61 
  src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
18f7a0af48d972d5186e9414475e080f1eb765f3 
  src/java/org/apache/sqoop/tool/ImportTool.java 
e9920058858653bec7407bf7992eb6445401e813 
  src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
8bdc3beb3677312ec0ee2e612616358bca4ca838 
  src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
PRE-CREATION 
  src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java PRE-CREATION 
  
src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
 PRE-CREATION 
  src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
PRE-CREATION 
  
src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
 PRE-CREATION 
  src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
  src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
PRE-CREATION 
  src/test/org/apache/sqoop/tool/TestImportTool.java 
1c0cf4d863692f75bb8831e834fae47fc18b5df5 


Diff: https://reviews.apache.org/r/66361/diff/3/

Changes: https://reviews.apache.org/r/66361/diff/2-3/


Testing
---

Ran unit and third party tests suite.


Thanks,

Szabolcs Vasas

Re: Review Request 66361: Implement HiveServer2 client

2018-04-10 Thread Szabolcs Vasas



> On April 6, 2018, 1:39 p.m., Boglarka Egyed wrote:
> > src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java
> > Lines 55 (patched)
> > 
> >
> > Why did you include only the message here not the exception itself?

The idea was that this is just getting the current user to log it so we might 
not want to log a full exception here if not successful, since the business 
logic itself could work well. But it makes sense to log the full trace, can be 
helpful to debug an issue.


- Szabolcs


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


On April 10, 2018, 9:40 a.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 10, 2018, 9:40 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> - I will upload the documentation soon
> 
> 
> Diffs
> -
> 
>   build.xml 7f68b573c65a61150ca78d158084586c87775d84 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 
> b7a25b7809e0d50166966a77161dc8ff603fb2d2 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> b02e4fe7fda25c7f8171c7db17d15a7987459687 
>   src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
> d259566180369a55d490144e6f865e728f4f2e61 
>   src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
> 18f7a0af48d972d5186e9414475e080f1eb765f3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> e9920058858653bec7407bf7992eb6445401e813 
>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
> 8bdc3beb3677312ec0ee2e612616358bca4ca838 
>   src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestImportTool.java 
> 1c0cf4d863692f75bb8831e834fae47fc18b5df5 
> 
> 
> Diff: https://reviews.apache.org/r/66361/diff/2/
> 
> 
> Testing
> ---
> 
> Ran unit and third party tests suite.
> 
> 
> Thanks,
> 
> Szabolcs Vasas
> 
>

Re: Review Request 66361: Implement HiveServer2 client

2018-04-10 Thread Szabolcs Vasas



> On April 6, 2018, 3:49 p.m., Fero Szabo wrote:
> > src/java/org/apache/sqoop/hive/HiveServer2Client.java
> > Lines 75 (patched)
> > 
> >
> > Is the package private visibility intentional?
> > 
> > Also applies for the remaining functions.

These are visible for testing, but I am considering refactoring this part and 
extract these methods to another class.


> On April 6, 2018, 3:49 p.m., Fero Szabo wrote:
> > src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java
> > Lines 43 (patched)
> > 
> >
> > There is a mismatch here between the parameter name in the constructor 
> > and the annotation (sqoopTool vs SqoopTool).
> > 
> > Obviously doesn't matter from the perspective of correctness, though, 
> > just bugs my eye. :)

Yepp, makes sense to keep the convention.


- Szabolcs


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


On April 10, 2018, 9:40 a.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated April 10, 2018, 9:40 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> - I will upload the documentation soon
> 
> 
> Diffs
> -
> 
>   build.xml 7f68b573c65a61150ca78d158084586c87775d84 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 
> b7a25b7809e0d50166966a77161dc8ff603fb2d2 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> b02e4fe7fda25c7f8171c7db17d15a7987459687 
>   src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
> d259566180369a55d490144e6f865e728f4f2e61 
>   src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
> 18f7a0af48d972d5186e9414475e080f1eb765f3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> e9920058858653bec7407bf7992eb6445401e813 
>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
> 8bdc3beb3677312ec0ee2e612616358bca4ca838 
>   src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestImportTool.java 
> 1c0cf4d863692f75bb8831e834fae47fc18b5df5 
> 
> 
> Diff:

Re: Review Request 66361: Implement HiveServer2 client

2018-04-10 Thread Szabolcs Vasas


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

(Updated April 10, 2018, 9:40 a.m.)


Review request for Sqoop.


Changes
---

Addressed review findings, extracted JdbcConnectionFactory decoration to 
AuthenticationConfiguration.


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


Repository: sqoop-trunk


Description
---

This JIRA covers the implementation of the client for HiveServer2 and its 
integration into the classes which use HiveImport.

- HiveClient interface is introduced with 2 implementation:
  - HiveImport: this is the original implementation which uses HiveCLI
  - HiveServer2Client: the new clients which connects to HS2 using JDBC 
connection
  - The common code is extracted to HiveCommon class
- HiveClient should be instantiated using HiveClientFactory which creates and 
configures the right HiveClient based on the configuration in SqoopOptions
- HiveMiniCluster is introduced with a couple of helper classes to enable 
end-to-end HS2 tests
- A couple of new options are added to SqoopOptions to be able to configure the 
connection to HS2
- Validation is implemented for these new options
- I will upload the documentation soon


Diffs (updated)
-

  build.xml 7f68b573c65a61150ca78d158084586c87775d84 
  ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
  src/java/org/apache/sqoop/SqoopOptions.java 
651cebd69ee7e75d06c75945e3607c4fab7eb11c 
  src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveImport.java 
c2729119d31f7e585f204f2d31b2051eea71b72b 
  src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java PRE-CREATION 
  src/java/org/apache/sqoop/hive/TableDefWriter.java 
b7a25b7809e0d50166966a77161dc8ff603fb2d2 
  src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
b02e4fe7fda25c7f8171c7db17d15a7987459687 
  src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
d259566180369a55d490144e6f865e728f4f2e61 
  src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
18f7a0af48d972d5186e9414475e080f1eb765f3 
  src/java/org/apache/sqoop/tool/ImportTool.java 
e9920058858653bec7407bf7992eb6445401e813 
  src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestHiveServer2TextImport.java PRE-CREATION 
  src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
8bdc3beb3677312ec0ee2e612616358bca4ca838 
  src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
PRE-CREATION 
  src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java PRE-CREATION 
  
src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
 PRE-CREATION 
  src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
PRE-CREATION 
  
src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
 PRE-CREATION 
  src/test/org/apache/sqoop/testutil/HiveServer2TestUtil.java PRE-CREATION 
  src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
PRE-CREATION 
  src/test/org/apache/sqoop/tool/TestImportTool.java 
1c0cf4d863692f75bb8831e834fae47fc18b5df5 


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

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


Testing
---

Ran unit and third party tests suite.


Thanks,

Szabolcs Vasas

Re: Review Request 66361: Implement HiveServer2 client

2018-04-06 Thread Fero Szabo via Review Board


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




src/java/org/apache/sqoop/hive/HiveServer2Client.java
Lines 75 (patched)


Is the package private visibility intentional?

Also applies for the remaining functions.



src/java/org/apache/sqoop/hive/TableDefWriter.java
Lines 300 (patched)


Is the package private visibility intentional?



src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java
Lines 43 (patched)


There is a mismatch here between the parameter name in the constructor and 
the annotation (sqoopTool vs SqoopTool).

Obviously doesn't matter from the perspective of correctness, though, just 
bugs my eye. :)


- Fero Szabo


On March 29, 2018, 3:15 p.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated March 29, 2018, 3:15 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> - I will upload the documentation soon
> 
> 
> Diffs
> -
> 
>   build.xml d85cf7198c5d7ca382407f927e73a85a17d12a62 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 
> b7a25b7809e0d50166966a77161dc8ff603fb2d2 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> b02e4fe7fda25c7f8171c7db17d15a7987459687 
>   src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
> d259566180369a55d490144e6f865e728f4f2e61 
>   src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
> 18f7a0af48d972d5186e9414475e080f1eb765f3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> e9920058858653bec7407bf7992eb6445401e813 
>   src/test/org/apache/sqoop/hive/TestHS2TextImport.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
> 8bdc3beb3677312ec0ee2e612616358bca4ca838 
>   src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/HS2TestUtil.java PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestImportTool.java 
> 1c0cf4d863692f75bb8831e834fae47fc18b5df5 
> 
> 
> Diff: https://reviews.apache.org/r/66361/diff/1/
> 
> 
> Testing
> ---
> 
> Ran unit and third party tests suite.
> 
> 
> Thanks,
> 
> Szabolcs Vasas
> 
>

Re: Review Request 66361: Implement HiveServer2 client

2018-04-06 Thread Boglarka Egyed


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




src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java
Lines 55 (patched)


Why did you include only the message here not the exception itself?



src/test/org/apache/sqoop/hive/TestHS2TextImport.java
Lines 1 (patched)


Missing Apache license header.



src/test/org/apache/sqoop/hive/TestHiveClientFactory.java
Lines 1 (patched)


Missing Apache license header.



src/test/org/apache/sqoop/hive/TestHiveServer2Client.java
Lines 1 (patched)


Missing Apache license header.



src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java
Lines 41 (patched)


Inconsistent naming convention: other HiveServer2 related tests contain HS2 
in their names except this. Also the business logic classes cointain 
HiveServer2 in their names so maybe not using HS2 would be more straightforward 
- what do you think?


- Boglarka Egyed


On March 29, 2018, 3:15 p.m., Szabolcs Vasas wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66361/
> ---
> 
> (Updated March 29, 2018, 3:15 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3309
> https://issues.apache.org/jira/browse/SQOOP-3309
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> ---
> 
> This JIRA covers the implementation of the client for HiveServer2 and its 
> integration into the classes which use HiveImport.
> 
> - HiveClient interface is introduced with 2 implementation:
>   - HiveImport: this is the original implementation which uses HiveCLI
>   - HiveServer2Client: the new clients which connects to HS2 using JDBC 
> connection
>   - The common code is extracted to HiveCommon class
> - HiveClient should be instantiated using HiveClientFactory which creates and 
> configures the right HiveClient based on the configuration in SqoopOptions
> - HiveMiniCluster is introduced with a couple of helper classes to enable 
> end-to-end HS2 tests
> - A couple of new options are added to SqoopOptions to be able to configure 
> the connection to HS2
> - Validation is implemented for these new options
> - I will upload the documentation soon
> 
> 
> Diffs
> -
> 
>   build.xml d85cf7198c5d7ca382407f927e73a85a17d12a62 
>   ivy.xml 6be4fa20fbbf1f303c69d86942b1874e18a14afc 
>   src/java/org/apache/sqoop/SqoopOptions.java 
> 651cebd69ee7e75d06c75945e3607c4fab7eb11c 
>   src/java/org/apache/sqoop/hive/HiveClient.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientCommon.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveClientFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveImport.java 
> c2729119d31f7e585f204f2d31b2051eea71b72b 
>   src/java/org/apache/sqoop/hive/HiveServer2Client.java PRE-CREATION 
>   src/java/org/apache/sqoop/hive/HiveServer2ConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/hive/TableDefWriter.java 
> b7a25b7809e0d50166966a77161dc8ff603fb2d2 
>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 
> b02e4fe7fda25c7f8171c7db17d15a7987459687 
>   src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 
> d259566180369a55d490144e6f865e728f4f2e61 
>   src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 
> 18f7a0af48d972d5186e9414475e080f1eb765f3 
>   src/java/org/apache/sqoop/tool/ImportTool.java 
> e9920058858653bec7407bf7992eb6445401e813 
>   src/test/org/apache/sqoop/hive/TestHS2TextImport.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveClientFactory.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveMiniCluster.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestHiveServer2Client.java PRE-CREATION 
>   src/test/org/apache/sqoop/hive/TestTableDefWriter.java 
> 8bdc3beb3677312ec0ee2e612616358bca4ca838 
>   src/test/org/apache/sqoop/hive/minicluster/AuthenticationConfiguration.java 
> PRE-CREATION 
>   src/test/org/apache/sqoop/hive/minicluster/HiveMiniCluster.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/KerberosAuthenticationConfiguration.java
>  PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/NoAuthenticationConfiguration.java 
> PRE-CREATION 
>   
> src/test/org/apache/sqoop/hive/minicluster/PasswordAuthenticationConfiguration.java
>  PRE-CREATION 
>   src/test/org/apache/sqoop/testutil/HS2TestUtil.java PRE-CREATION 
>   src/test/org/apache/sqoop/tool/TestHiveServer2OptionValidations.java 
> PRE-CREATION 
>

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

Re: Review Request 66361: Implement HiveServer2 client

20 matches

Site Navigation

Mail list logo

Footer information