Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/14000 )
Change subject: Docker image for python kudu client ...................................................................... Patch Set 2: (13 comments) http://gerrit.cloudera.org:8080/#/c/14000/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/14000/2//COMMIT_MSG@7 PS2, Line 7: Docker image for python kudu client Add KUDU-2849 http://gerrit.cloudera.org:8080/#/c/14000/2//COMMIT_MSG@9 PS2, Line 9: A base image that has all the development libraries required Adjust this line to match the description based on my comments in the dockerfile. http://gerrit.cloudera.org:8080/#/c/14000/2//COMMIT_MSG@14 PS2, Line 14: Set Env Nit: trailing space http://gerrit.cloudera.org:8080/#/c/14000/2/docker/Dockerfile File docker/Dockerfile: http://gerrit.cloudera.org:8080/#/c/14000/2/docker/Dockerfile@222 PS2, Line 222: # Builds a base image that has all the development libraries required to run Python Kudu Client. Maybe "Builds a runtime image with the Kudu python client pre-installed." http://gerrit.cloudera.org:8080/#/c/14000/2/docker/Dockerfile@224 PS2, Line 224: ARG BASE_OS I don't think you need this here. http://gerrit.cloudera.org:8080/#/c/14000/2/docker/Dockerfile@225 PS2, Line 225: FROM build AS kudu-python The build image is massive. This doesn't work with the runtime image because it's missing the python stuff. Maybe try using the dev image. If that doesn't work or is too large we may need to adjust the bootstrap scripts so we can just bootstrap the python stuff needed. http://gerrit.cloudera.org:8080/#/c/14000/2/docker/Dockerfile@227 PS2, Line 227: nit: extra line http://gerrit.cloudera.org:8080/#/c/14000/2/docker/Dockerfile@230 PS2, Line 230: ENV LD_LIBRARY_PATH=$LD_LIBRARY_PATH:/usr/local/lib64 nit: move this above the workdir so it can be cached through changes. http://gerrit.cloudera.org:8080/#/c/14000/2/docker/Dockerfile@232 PS2, Line 232: RUN pip install kudu-python-*.tar.gz Once we install the client can we removed the original `kudu-python-*.tar.gz` to reduce the image size? Ideally this would be done in the same RUN step with &&. http://gerrit.cloudera.org:8080/#/c/14000/2/docker/Dockerfile@234 PS2, Line 234: nit: extra line http://gerrit.cloudera.org:8080/#/c/14000/2/docker/Dockerfile@242 PS2, Line 242: LABEL org.label-schema.name="Apache Kudu Python Client Base" \ Maybe remove "Base", since this image is intended to be used/run. http://gerrit.cloudera.org:8080/#/c/14000/2/docker/Dockerfile@243 PS2, Line 243: org.label-schema.description="A base image that has all the Kudu Python \ Update this to match the description above. http://gerrit.cloudera.org:8080/#/c/14000/2/docker/README.adoc File docker/README.adoc: http://gerrit.cloudera.org:8080/#/c/14000/2/docker/README.adoc@113 PS2, Line 113: To run Kudu Python client image: I don't think you need the extra run guidance here given the other images don't have it. It's also covered in "Running an image" above. -- To view, visit http://gerrit.cloudera.org:8080/14000 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7cab8ffe114c73752c261f27ebc7a58e4b57a6e Gerrit-Change-Number: 14000 Gerrit-PatchSet: 2 Gerrit-Owner: Sandish Kumar HN <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 05 Aug 2019 03:08:08 +0000 Gerrit-HasComments: Yes
