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

Reply via email to