Thomas Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/13248 )
Change subject: IMPALA-8503: add option to start Kudu cluster with HMS integration ...................................................................... Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/node_templates/common/etc/init.d/kudu-master File testdata/cluster/node_templates/common/etc/init.d/kudu-master: http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/node_templates/common/etc/init.d/kudu-master@31 PS1, Line 31: KUDU_COMMON_ARGS+=("-hive_metastore_uris=thrift://${INTERNAL_LISTEN_HOST}:9083") Instead of doing all of the work of piping an argument all the way through testdata/cluster/admin and having this logic here, I wonder if it would be easier just to add an env variable like EXTRA_KUDU_STARTUP_ARGS or whatever that if its set we always just append it to KUDU_COMMON_ARGS here. It also doesn't look like this patch actually uses the functionality that you've added here, unless I'm missing something. It might be easier for reviewers to understand how you intend for this to work if you include it in a patch along with a test that actually exercises this functionality -- To view, visit http://gerrit.cloudera.org:8080/13248 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I734d14ede6a03ad52e820e38a1fbcbac0a40ede2 Gerrit-Change-Number: 13248 Gerrit-PatchSet: 1 Gerrit-Owner: Hao Hao <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Thomas Marshall <[email protected]> Gerrit-Comment-Date: Mon, 06 May 2019 22:20:17 +0000 Gerrit-HasComments: Yes
