Jim Apple has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/11731 )

Change subject: IMPALA-7698: Add centos support to bootstrap_system.
......................................................................


Patch Set 1:

(6 comments)

Phil and Laszlo: thanks so much for doing this!

http://gerrit.cloudera.org:8080/#/c/11731/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/11731/1//COMMIT_MSG@33
PS1, Line 33: centos:7 is not addressed by this change. The move to systemd 
makes "service
nit: long lines


http://gerrit.cloudera.org:8080/#/c/11731/1/bin/bootstrap_system.sh
File bin/bootstrap_system.sh:

http://gerrit.cloudera.org:8080/#/c/11731/1/bin/bootstrap_system.sh@104
PS1, Line 104: REAL_APT_GET=$(ubuntu which apt-get)
Do you want to do something similar for yum? IIRC, i wrote this block because 
in a non-interactive setting, I expect that connectivity may be temporarily 
down, and that's not a very informative way for a build to fail.


http://gerrit.cloudera.org:8080/#/c/11731/1/bin/bootstrap_system.sh@143
PS1, Line 143: redhat sudo wget -nv 
http://www-us.apache.org/dist/maven/maven-3/3.5.4/binaries/apache-maven-3.5.4-bin.tar.gz
 http://www-us.apache.org/dist/ant/binaries/apache-ant-1.9.13-bin.tar.gz
> line too long (181 > 90)
httpS and check signatures and checksums?


http://gerrit.cloudera.org:8080/#/c/11731/1/bin/bootstrap_system.sh@194
PS1, Line 194: # redhat sudo bash -c "echo host all all 127.0.0.1/32 md5 >> 
/var/lib/pgsql/data/pg_hba.conf"
> line too long (93 > 90)
I think this line was accidentally left in.

Also, what are the security implications of this compared to what we do on 
Ubuntu? Is this way more secure?


http://gerrit.cloudera.org:8080/#/c/11731/1/bin/bootstrap_system.sh@274
PS1, Line 274:   SET_JAVA_HOME="export JAVA_HOME=$(echo 
/usr/lib/jvm/java-1.8.0-openjdk-* | head -n 1)"
Does this not work on Ubuntu, too?

Do you want to warn if there is more than one?


http://gerrit.cloudera.org:8080/#/c/11731/1/docker/entrypoint.sh
File docker/entrypoint.sh:

http://gerrit.cloudera.org:8080/#/c/11731/1/docker/entrypoint.sh@81
PS1, Line 81:       adduser --uid $1 impdev
Do you want to add a comment here about why the command is different for Ubuntu?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id54294d7607f51de87a9de373dcfc4a33f4bedf5
Gerrit-Change-Number: 11731
Gerrit-PatchSet: 1
Gerrit-Owner: Philip Zeyliger <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Jim Apple <[email protected]>
Gerrit-Reviewer: Joe McDonnell <[email protected]>
Gerrit-Reviewer: Laszlo Gaal <[email protected]>
Gerrit-Reviewer: Philip Zeyliger <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Comment-Date: Sat, 20 Oct 2018 14:02:42 +0000
Gerrit-HasComments: Yes

Reply via email to