Alexey Serbin has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/18049 )

Change subject: Fix readlink behaviour on MacOS in start_kudu.sh
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18049/1/src/kudu/scripts/start_kudu.sh
File src/kudu/scripts/start_kudu.sh:

http://gerrit.cloudera.org:8080/#/c/18049/1/src/kudu/scripts/start_kudu.sh@78
PS1, Line 78:   echo $(readlink -f $(dirname $0))
> I think we would only need the realpath if we are running the script from a
It seems it's assumed to run the script from the build directory if not 
providing -b/--builddir command-line option, right?

I think it would help if that assumption is explicitly stated in the help/usage 
output for this script.

BTW, the essential part of the script are the binaries of kudu-master and 
kudu-tserver, and webserver doc root isn't a prerequisite to run a Kudu 
mini-cluster, IIUC.  In addition, kudu-master and kudu-tserver do automatically 
find the proper location of the docroot if KUDU_HOME environment variable is 
set.

With that, I'd probably try to explore the following updates to the script:

* remove the usage of realpath at all since everything is capable of running on 
symlinked paths, IIUC
* add a mention that the script if assumed to run from the build directory 
unless -b/--builddir option is specified
* set WEBSERVER_DOC_ROOT to $BUILDDIR/../../www only if $KUDU_HOME isn't 
defined/empty or $KUDU_HOME/www isn't available
* don't require the presence of the web server doc root: just issue a warning 
if it's not available either as $BUILDDIR/../../www or as $KUDU_HOME/www
* don't set the --webserver_doc_root flag for kudu-master and kudu-tserver if 
WEBSERVER_DOC_ROOT directory as defined above doesn't exit

Would it make sense?



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0057a6d6668488001be8b4f74f46010e2620b46e
Gerrit-Change-Number: 18049
Gerrit-PatchSet: 1
Gerrit-Owner: Zoltan Chovan <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Zoltan Chovan <[email protected]>
Gerrit-Comment-Date: Wed, 24 Nov 2021 20:19:14 +0000
Gerrit-HasComments: Yes

Reply via email to