impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bharath Vissapragada (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5184: build fe against both Hive 1 & 2 APIs
Date Tue, 11 Apr 2017 18:02:42 GMT
Bharath Vissapragada has posted comments on this change.

Change subject: IMPALA-5184: build fe against both Hive 1 & 2 APIs
......................................................................


Patch Set 8: Code-Review+1

(6 comments)

http://gerrit.cloudera.org:8080/#/c/5538/7/bin/impala-config.sh
File bin/impala-config.sh:

Line 377: # a complete Hive build.
> I ran into the dreaded sticky config variable problem with some of these va
Thanks for fixing it.


http://gerrit.cloudera.org:8080/#/c/5538/7/common/thrift/.gitignore
File common/thrift/.gitignore:

Line 4: hive-2-api/TCLIService.thrift
> The other one is checked in, so any changes to it shouldn't be ignored.
Ok I wasn't aware of that. Thanks for clarifying.


http://gerrit.cloudera.org:8080/#/c/5538/7/common/thrift/CMakeLists.txt
File common/thrift/CMakeLists.txt:

Line 189: # The thrift-generated java classes defined in TCLIService are also downloaded via
maven.
> TCLIService.thrift from Hive 2 adds additional RPC endpoints that we don't 
Makes sense. My only concern was that if the spec is updated, we may have to fix this logic
again. However that looks less likely, so fine by me.


http://gerrit.cloudera.org:8080/#/c/5538/8/fe/src/compat-hive-1/java/org/apache/hive/service/rpc/thrift/TGetTablesReq.java
File fe/src/compat-hive-1/java/org/apache/hive/service/rpc/thrift/TGetTablesReq.java:

Line 1: package org.apache.hive.service.rpc.thrift;
Apache license in the newly added classes?


http://gerrit.cloudera.org:8080/#/c/5538/8/fe/src/compat-hive-1/java/org/apache/impala/compat/MetaStoreShim.java
File fe/src/compat-hive-1/java/org/apache/impala/compat/MetaStoreShim.java:

Line 16: // under the License.
nit: \n after license.


http://gerrit.cloudera.org:8080/#/c/5538/7/fe/src/compat-hive-2/java/org/apache/impala/compat/MetaStoreCompatShim.java
File fe/src/compat-hive-2/java/org/apache/impala/compat/MetaStoreCompatShim.java:

Line 100
> The thrift classes moved between packages. The code is identical but the cl
Ah, got it.


-- 
To view, visit http://gerrit.cloudera.org:8080/5538
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifbc265281c04fe3136bc3c920dbac966742ce09a
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message