Alexey Serbin 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:

(6 comments)

I don't have enough context for high-level review of this changelist, but it 
seems the comment that Thomas added makes sense.

Also, what is the way to configure the way how services started via these 
scripts?  I know that in case of UNIX init.d scripts there are configuration 
files in /etc that are picked up by the scripts.  Using environment variables 
makes it look like something similar to that approach.

http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/admin
File testdata/cluster/admin:

http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/admin@425
PS1, Line 425: ;
drop


http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/admin@427
PS1, Line 427: ;
drop


http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/admin@428
PS1, Line 428: ;
drop


http://gerrit.cloudera.org:8080/#/c/13248/1/testdata/cluster/admin@497
PS1, Line 497: function restart {
             :   if [[ $# -ne 1 ]]; then
             :     echo restart must be called with a single argument -- the 
service to restart. 1>&2
             :     exit 1
             :   fi
             :   local SERVICE=$1
             :   restart_with_arg $SERVICE
             : }
             :
             : function restart_with_arg {
             :   local SERVICE=$1
             :   shift
             :   local ARG="${1:-}"
             :   stop $SERVICE
             :   start_with_arg $SERVICE $ARG
             : }
Did you verify this works as expected?  I'm a bit confused about check at L498 
and the code at L508-L509?


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: :9083
nit: if that's the default and cannot be configured, maybe drop it?


http://gerrit.cloudera.org:8080/#/c/13248/2/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/2/testdata/cluster/node_templates/common/etc/init.d/kudu-master@32
PS2, Line 32: ;
nit: drop



--
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: Adar Dembo <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Hao Hao <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Thomas Marshall <[email protected]>
Gerrit-Comment-Date: Tue, 14 May 2019 00:55:00 +0000
Gerrit-HasComments: Yes

Reply via email to