You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by daniel voros <da...@gmail.com> on 2018/06/21 13:39:44 UTC
Review Request 67689: Use hive executable in (non-JDBC) Hive imports
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67689/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-3323
https://issues.apache.org/jira/browse/SQOOP-3323
Repository: sqoop-trunk
Description
-------
When doing Hive imports the old way (not via JDBC that was introduced in SQOOP-3309) we're trying to use the CliDriver class from Hive and fall back to the hive executable (a.k.a. Hive Cli) if that class is not found.
Since CliDriver and the hive executable that's relying on it are deprecated (see also HIVE-10511), we should switch to using beeline to talk to Hive. With recent additions (e.g. HIVE-18963) this should be easier than before.
As a first step we could switch to using hive executable. With HIVE-19728 it will be possible (in Hive 3.1) to configure hive to actually run beeline when using the hive executable. This way we could leave it to the user to decide whether to use the deprecated cli or use beeline instead.
Diffs
-----
src/java/org/apache/sqoop/hive/HiveImport.java 5da00a74
src/test/org/apache/sqoop/TestIncrementalImport.java 1ab98021
src/test/org/apache/sqoop/TestSqoopJobDataPublisher.java b3579ac1
src/test/org/apache/sqoop/hive/TestHiveImport.java 436f0e51
src/test/org/apache/sqoop/manager/postgresql/PostgresqlExternalTableImportTest.java dd4cfb48
Diff: https://reviews.apache.org/r/67689/diff/1/
Testing
-------
run thirdparty and normal UTs, also tested on a cluster
I'm removing PostgresqlExternalTableImportTest since it was relying on the CliDriver path to do an actual Hive import.
Thanks,
daniel voros
Re: Review Request 67689: Use hive executable in (non-JDBC) Hive
imports
Posted by Fero Szabo via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67689/#review205234
-----------------------------------------------------------
Hi Dani,
I had a look at your patch and it basically looks good to me, it applied cleanly on my system and all tests passed.
My only concern is that we lose a bit of test coverage. Wouldn't it make sense to reimplement the testcase you deleted in a different way? As far as I can see, it was the only test for external hive tables...
It might take some effort to do this though, I didn't have time to understand how it works exactly. One might be able to reuse the code in the TestHiveImport class.
src/java/org/apache/sqoop/hive/HiveImport.java
Line 330 (original)
<https://reviews.apache.org/r/67689/#comment288167>
Was this config effectively only used in testing and no longer needed?
- Fero Szabo
On June 21, 2018, 1:39 p.m., daniel voros wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67689/
> -----------------------------------------------------------
>
> (Updated June 21, 2018, 1:39 p.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-3323
> https://issues.apache.org/jira/browse/SQOOP-3323
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> When doing Hive imports the old way (not via JDBC that was introduced in SQOOP-3309) we're trying to use the CliDriver class from Hive and fall back to the hive executable (a.k.a. Hive Cli) if that class is not found.
>
> Since CliDriver and the hive executable that's relying on it are deprecated (see also HIVE-10511), we should switch to using beeline to talk to Hive. With recent additions (e.g. HIVE-18963) this should be easier than before.
>
> As a first step we could switch to using hive executable. With HIVE-19728 it will be possible (in Hive 3.1) to configure hive to actually run beeline when using the hive executable. This way we could leave it to the user to decide whether to use the deprecated cli or use beeline instead.
>
>
> Diffs
> -----
>
> src/java/org/apache/sqoop/hive/HiveImport.java 5da00a74
> src/test/org/apache/sqoop/TestIncrementalImport.java 1ab98021
> src/test/org/apache/sqoop/TestSqoopJobDataPublisher.java b3579ac1
> src/test/org/apache/sqoop/hive/TestHiveImport.java 436f0e51
> src/test/org/apache/sqoop/manager/postgresql/PostgresqlExternalTableImportTest.java dd4cfb48
>
>
> Diff: https://reviews.apache.org/r/67689/diff/1/
>
>
> Testing
> -------
>
> run thirdparty and normal UTs, also tested on a cluster
>
> I'm removing PostgresqlExternalTableImportTest since it was relying on the CliDriver path to do an actual Hive import.
>
>
> Thanks,
>
> daniel voros
>
>