[GitHub] larroy commented on a change in pull request #12791: [MXNET-1093] Add python3 Docker images for each MXNet release

[GitBox <gi...@apache.org>]

larroy commented on a change in pull request #12791: [MXNET-1093] Add python3 Docker images for each MXNet release
URL: https://github.com/apache/incubator-mxnet/pull/12791#discussion_r230054109
 
 

 ##########
 File path: docker/docker-python/build_python_dockerfile.sh
 ##########
 @@ -39,29 +40,34 @@ fi
 # Two params provided
 echo "Building Docker Images for Apache MXNet (Incubating) v$1"
 mxnet_version="${1}"
-test_dir="${2}"
+pip_tag="${2}"
+test_dir="${3}"
 
-docker_build_image(){
-    echo "Building docker image mxnet/python:${1}"
-    docker build -t mxnet/python:${1} -f ${2} .
-}
+# Remove the logs directory if it already exists else it may fail due to old logs.
+LOGDIR=~/temp/docker_logs
+if [ -d "${LOGDIR}" ]; then
+  rm -rf ${LOGDIR}
+fi
+mkdir ${LOGDIR}
 
-docker_tag_image(){
-    docker tag mxnet/python:${1} mxnet/python:${2}
-}
 
+# Functions
 docker_test_image_cpu(){
 
 Review comment:
   We can't have docker running like this because it leaves containers hanging around even when the parent process terminates. We introduce logic to avoid this kinds of problems in CI  https://github.com/apache/incubator-mxnet/pull/12381
   
   My suggestion is to use one of this two options:
   - Use https://github.com/apache/incubator-mxnet/blob/master/ci/build.py as it's done in https://github.com/apache/incubator-mxnet/blob/master/ci/docker_cache.py to run the tests safely, build the images in parallel. This is probably the fastest route that is safe but introduces some coupling between CI and CD.
   - Abstract the cleanup logic in build.py in a module which is then used from CI and CD with the specific commands.
   - Duplicate the logic from build.py and docker_cache.py for this task (copy & paste) and adjust for the specific case. A downside is that introduces similar logic to be maintained but not coupling.
   
   If on doubt, we can choose option 1 and go from there.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services