You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/03/19 16:07:32 UTC

[GitHub] [tvm] leandron opened a new pull request #7707: [CI] Improve docker/build.sh to accept a docker tag parameter.

leandron opened a new pull request #7707:
URL: https://github.com/apache/tvm/pull/7707


    Improve docker/build.sh to accept a docker tag parameter
    * This adds a new `--dockertag` parameter so that we can build docker images on a particular tag, not only `:latest` as given by Docker
    * It opens up the possibility of generating "staging" images on a different tag, in the same servers as we keep the production images
    * By default it keeps previous behaviour of using `:latest` tag.
   
   cc @areusch @tmoreau89 @u99127 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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



[GitHub] [tvm] areusch commented on a change in pull request #7707: [CI] Improve docker/build.sh to accept a docker tag parameter.

Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #7707:
URL: https://github.com/apache/tvm/pull/7707#discussion_r597816150



##########
File path: docker/build.sh
##########
@@ -114,9 +124,10 @@ function upsearch () {
 # reasonable defaults if you run it outside of Jenkins.
 WORKSPACE="${WORKSPACE:-${SCRIPT_DIR}/../}"
 BUILD_TAG="${BUILD_TAG:-tvm}"
+DOCKER_IMAGE_TAG="${DOCKER_IMAGE_TAG:-latest}"
 
 # Determine the docker image name
-DOCKER_IMG_NAME="${BUILD_TAG}.${CONTAINER_TYPE}"
+DOCKER_IMG_NAME="${BUILD_TAG}.${CONTAINER_TYPE}:${DOCKER_IMAGE_TAG}"

Review comment:
       maybe want to do this after sanitizing the DOCKER_IMG_NAME below? or maybe more ideally assemble DOCKER_IMG_SPEC and use that in place of DOCKER_IMG_NAME on the command-line below

##########
File path: docker/build.sh
##########
@@ -45,6 +49,12 @@ shift 1
 DOCKERFILE_PATH="${SCRIPT_DIR}/Dockerfile.${CONTAINER_TYPE}"
 DOCKER_CONTEXT_PATH="${SCRIPT_DIR}"
 
+if [[ "$1" == "--dockertag" ]]; then

Review comment:
       not sure the "docker" prefix is necessary, since this script is already `docker/build.sh`.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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



[GitHub] [tvm] leandron commented on a change in pull request #7707: [CI] Improve docker/build.sh to accept a docker tag parameter.

Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #7707:
URL: https://github.com/apache/tvm/pull/7707#discussion_r597851618



##########
File path: docker/build.sh
##########
@@ -114,9 +124,10 @@ function upsearch () {
 # reasonable defaults if you run it outside of Jenkins.
 WORKSPACE="${WORKSPACE:-${SCRIPT_DIR}/../}"
 BUILD_TAG="${BUILD_TAG:-tvm}"
+DOCKER_IMAGE_TAG="${DOCKER_IMAGE_TAG:-latest}"
 
 # Determine the docker image name
-DOCKER_IMG_NAME="${BUILD_TAG}.${CONTAINER_TYPE}"
+DOCKER_IMG_NAME="${BUILD_TAG}.${CONTAINER_TYPE}:${DOCKER_IMAGE_TAG}"

Review comment:
       I think I made this change. Have a look.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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



[GitHub] [tvm] leandron commented on pull request #7707: [CI] Improve docker/build.sh to accept a docker tag parameter.

Posted by GitBox <gi...@apache.org>.
leandron commented on pull request #7707:
URL: https://github.com/apache/tvm/pull/7707#issuecomment-802982166


   > @leandron thanks for the PR! for the staging images, this would still generate e.g. tvm.ci_arm:v0.03. for building staging images to test in TVM CI, we would still need to do:
   
   This is basically just to enable having images on different tags when running `docker/build.sh`. Yes, I use it for my own tests, and I'm aware that there are other steps currently to update the images for the production CI.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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



[GitHub] [tvm] leandron commented on a change in pull request #7707: [CI] Improve docker/build.sh to accept a docker tag parameter.

Posted by GitBox <gi...@apache.org>.
leandron commented on a change in pull request #7707:
URL: https://github.com/apache/tvm/pull/7707#discussion_r597842942



##########
File path: docker/build.sh
##########
@@ -45,6 +49,12 @@ shift 1
 DOCKERFILE_PATH="${SCRIPT_DIR}/Dockerfile.${CONTAINER_TYPE}"
 DOCKER_CONTEXT_PATH="${SCRIPT_DIR}"
 
+if [[ "$1" == "--dockertag" ]]; then

Review comment:
       Agreed. I'll remove.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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



[GitHub] [tvm] areusch merged pull request #7707: [CI] Improve docker/build.sh to accept a docker tag parameter.

Posted by GitBox <gi...@apache.org>.
areusch merged pull request #7707:
URL: https://github.com/apache/tvm/pull/7707


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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



[GitHub] [tvm] leandron commented on pull request #7707: [CI] Improve docker/build.sh to accept a docker tag parameter.

Posted by GitBox <gi...@apache.org>.
leandron commented on pull request #7707:
URL: https://github.com/apache/tvm/pull/7707#issuecomment-804207542


   @areusch I guess this can be merged, when you have some time.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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



[GitHub] [tvm] areusch commented on pull request #7707: [CI] Improve docker/build.sh to accept a docker tag parameter.

Posted by GitBox <gi...@apache.org>.
areusch commented on pull request #7707:
URL: https://github.com/apache/tvm/pull/7707#issuecomment-804215522


   thanks @leandron the PR is now merged!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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