Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12173 )
Change subject: [docker] Add an initial docker build ...................................................................... Patch Set 2: (10 comments) http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG@7 PS1, Line 7: an in > initial Done http://gerrit.cloudera.org:8080/#/c/12173/1//COMMIT_MSG@20 PS1, Line 20: - muti-os tools like build_mini_cluster_binaries.sh > Is this an incomplete thought or is the apostrophe in ENTRYPOINT unnecessar just a typo. http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile File docker/Dockerfile: http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@1 PS1, Line 1: # Licensed to the Apache Software Foundation (ASF) under one > Nit: remove empty line? Done http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@22 PS1, Line 22: ocs.dock > editing Done http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@27 PS1, Line 27: > Nit: two spaces here Done http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@35 PS1, Line 35: COPY ./docker/bootstrap-env.sh / > Curious why you chose to start with CentOS 7.6 and something like 6.6 (whic I planned to start with current/common versions of CentOS and Ubuntu for the sake of not dealing with older version issues while fleshing out the build. http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@50 PS1, Line 50: org.label-schema.dockerfile=$DOCKERFILE \ > Nit: looks like you're listing one dependency per line except here and in a Done http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@88 PS1, Line 88: org.label-schema.url=$URL \ > Does COPY operate recursively? Will it grab thirdparty/patches, for example Yes, copy will grab everything in/below this directory. http://gerrit.cloudera.org:8080/#/c/12173/1/docker/Dockerfile@100 PS1, Line 100: FROM kudu-thirdparty AS kudu-build > Is it safe to run `nproc` here to derive the parallelism? It might be, I think that impacts the ability for the image to be cached though because different nproc results can invalidate an image. http://gerrit.cloudera.org:8080/#/c/12173/1/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: http://gerrit.cloudera.org:8080/#/c/12173/1/thirdparty/build-definitions.sh@746 PS1, Line 746: mkdir -p $TP_DIR/../www/ > Curious why this is needed: the 'www' subdirectory is checked into source c I need this because in the docker build I copy and build thirdparty first without copying the rest or the source. -- To view, visit http://gerrit.cloudera.org:8080/12173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I95497b39e47f07301be75cbadd814656c7e2ea42 Gerrit-Change-Number: 12173 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Tue, 22 Jan 2019 04:26:44 +0000 Gerrit-HasComments: Yes
