sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Fero Szabo via Review Board <nore...@reviews.apache.org>
Subject Re: Review Request 67689: Use hive executable in (non-JDBC) Hive imports
Date Fri, 22 Jun 2018 12:42:28 GMT

-----------------------------------------------------------
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
> 
>


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