Joe McDonnell has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14363 )

Change subject: IMPALA-9005: distcc server bootstrapping improvements
......................................................................


Patch Set 1:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/14363/1/bin/distcc/distcc_server_bootstrap.sh
File bin/distcc/distcc_server_bootstrap.sh:

http://gerrit.cloudera.org:8080/#/c/14363/1/bin/distcc/distcc_server_bootstrap.sh@26
PS1, Line 26: Arguments are forwarded to
            : # distcc_server_setup.sh to specify IP ranges, etc.
Nit: Are we ok with the defaults for CCACHE_DIR/CCACHE_SIZE that are set in 
distcc_server_setup.sh? Is this something we should mention as a configurable 
parameter?


http://gerrit.cloudera.org:8080/#/c/14363/1/bin/distcc/distcc_server_bootstrap.sh@62
PS1, Line 62:   # Set HOME as workaround for ccache trying to access /.ccache
            :   HOME=$(pwd)
To check my understanding: this is ccache usage from these setup commands and 
not really important for later. It looks like bootstrap_virtualenv.py invokes 
ccache. If this is right, can you add a comment saying that this only applies 
to these setup commands?

Nit: if we are setting HOME directly, then maybe we don't need the -H on the 
sudo command?


http://gerrit.cloudera.org:8080/#/c/14363/1/bin/distcc/distcc_server_bootstrap.sh@70
PS1, Line 70:  . bin/impala-config.sh
            :  ./infra/python/deps/download_requirements
            :  DOWNLOAD_CDH_COMPONENTS=false ./bin/bootstrap_toolchain.py
Super-nit: indentation here is a single space, but it should be two spaces



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9e382e90f68a5a2fe2917184fa4603bed492a308
Gerrit-Change-Number: 14363
Gerrit-PatchSet: 1
Gerrit-Owner: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Comment-Date: Thu, 03 Oct 2019 23:07:26 +0000
Gerrit-HasComments: Yes

Reply via email to