[GitHub] marcoabreu commented on a change in pull request #9946: [DO NOT MERGE] Start CI docker revamp
marcoabreu commented on a change in pull request #9946: [DO NOT MERGE] Start CI docker revamp URL: https://github.com/apache/incubator-mxnet/pull/9946#discussion_r172190039 ## File path: ci/build.py ## @@ -69,7 +69,7 @@ def build_docker(platform: str, docker_binary: str) -> None: cmd = [docker_binary, "build", "-f", get_dockerfile(platform), "-t", tag, -"."] +"docker"] Review comment: The problem here is that "." requires the paths in the Dockerfiles to be relative to ci/ instead of ci/docker. Maybe I'll update it to be relative to the parent dir of ``get_dockerfile(platform)``. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #9946: [DO NOT MERGE] Start CI docker revamp
marcoabreu commented on a change in pull request #9946: [DO NOT MERGE] Start CI docker revamp URL: https://github.com/apache/incubator-mxnet/pull/9946#discussion_r171937930 ## File path: tests/ci_build/Dockerfile.cpu_clang ## @@ -1,21 +0,0 @@ -FROM ubuntu:16.04 - -COPY install/ubuntu_install_core.sh /install/ Review comment: Thanks a lot for your suggestion. I have made the same experience and I will probably switch back to having every function separately. The runtime scripts (called build_functions.sh) will stay as one file while install_functions.sh will be split into separate files again. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #9946: [DO NOT MERGE] Start CI docker revamp
marcoabreu commented on a change in pull request #9946: [DO NOT MERGE] Start CI docker revamp URL: https://github.com/apache/incubator-mxnet/pull/9946#discussion_r171728648 ## File path: tests/ci_build/Dockerfile.cpu_clang ## @@ -1,21 +0,0 @@ -FROM ubuntu:16.04 - -COPY install/ubuntu_install_core.sh /install/ Review comment: I agree. I'll check if I might be able to reduce the number of Dockerfiles and thus make centralization obsolete. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] marcoabreu commented on a change in pull request #9946: [DO NOT MERGE] Start CI docker revamp
marcoabreu commented on a change in pull request #9946: [DO NOT MERGE] Start CI docker revamp URL: https://github.com/apache/incubator-mxnet/pull/9946#discussion_r171719266 ## File path: tests/ci_build/Dockerfile.cpu_clang ## @@ -1,21 +0,0 @@ -FROM ubuntu:16.04 - -COPY install/ubuntu_install_core.sh /install/ Review comment: We plan to make use of a centralized docker repository for all our slaves as part of Auto Scaling, thus reducing the impact. But you're right, a single change will trigger rebuilds for all containers. 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: us...@infra.apache.org With regards, Apache Git Services
