Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/17015 )
Change subject: IMPALA-10389: impala-profile-tool container ...................................................................... Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/17015/1/docker/CMakeLists.txt File docker/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/17015/1/docker/CMakeLists.txt@132 PS1, Line 132: file(WRITE ${CMAKE_SOURCE_DIR}/docker/docker-images.txt "${exported_image_names}") > I'm wondering whether it is significant that the profile tool image is adde You're right, this should go at the end. I didn't check the contents of the file. http://gerrit.cloudera.org:8080/#/c/17015/1/docker/CMakeLists.txt@172 PS1, Line 172: setup_utility_build_context.py > I think this should be setup_build_context.py? Done http://gerrit.cloudera.org:8080/#/c/17015/1/docker/CMakeLists.txt@184 PS1, Line 184: WORKING_DIRECTORY ${IMPALA_UTILITY_BUILD_CONTEXT_DIR}/${build_type} > Nit: indentation should line up with COMMAND Done http://gerrit.cloudera.org:8080/#/c/17015/1/docker/CMakeLists.txt@186 PS1, Line 186: #DEPENDS ${CMAKE_SOURCE_DIR}/bin/graceful_shutdown_backends.sh > Nit: stray line? Missing the entrypoint script. -- To view, visit http://gerrit.cloudera.org:8080/17015 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36915cd686ab930dcc934bc0c81bff8c16d46714 Gerrit-Change-Number: 17015 Gerrit-PatchSet: 1 Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Fri, 05 Feb 2021 00:39:36 +0000 Gerrit-HasComments: Yes