Dan Burkert has posted comments on this change. Change subject: Add basic Hive MetaStore client ......................................................................
Patch Set 5: (5 comments) http://gerrit.cloudera.org:8080/#/c/7053/5/src/kudu/hms/hive_metastore-test.cc File src/kudu/hms/hive_metastore-test.cc: Line 23: // See https://stackoverflow.com/questions/1156003/c-namespace-collision-with-gtest-and-boost > Perhaps you can encapsulate the workaround in a new test-only header then? I want to punt on this until we have two cases where it's necessary, I'm still not 100% sure of exactly the header order necessary to have this issue. http://gerrit.cloudera.org:8080/#/c/7053/5/src/kudu/hms/mini_hms.cc File src/kudu/hms/mini_hms.cc: Line 77: Env* env = Env::Default(); > I know, nor is it how ExternalMiniCluster works, but that doesn't mean we c Why is it better? It forces every caller to call Env::Default() instead. Line 109: return Status::RuntimeError("schematool failed"); > Perhaps, but "exit status" is a real thing that I know how to correlate to I've changed it to get the real exit code, but I'm still not printing it because I don't think it's useful. schema tool and hive loudly log errors. http://gerrit.cloudera.org:8080/#/c/7053/5/thirdparty/download-thirdparty.sh File thirdparty/download-thirdparty.sh: Line 323: if [ ! -d "$THRIFT_SOURCE" ]; then > Alright, but you're on the hook for any obscure autotools version mismatch I've switched thrift to use cmake instead. Bison continues to not call autoreconf since I can't get it to work on all platforms. Line 342: # Remove the share/doc dir which contains 2GiB of HTML project documentation. > What I meant was: we're comfortable building the combined LLVM tarball ours We are forced to do that because LLVM doesn't a unified tarball with the components we need. I'd prefer not to mess with tarballs when we don't need to, since it makes it much easier to trace provenance. The .html documents compress fairly well, so the actual tarball isn't inflated by 2GiB. -- To view, visit http://gerrit.cloudera.org:8080/7053 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I155223da912bc18a759df2f1f6bc25d1132a99ee Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Adar Dembo <a...@cloudera.com> Gerrit-Reviewer: Dan Burkert <danburk...@apache.org> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
