josephevans commented on a change in pull request #19654:
URL: https://github.com/apache/incubator-mxnet/pull/19654#discussion_r573428934
##########
File path: ci/build.py
##########
@@ -117,6 +123,8 @@ def run_cmd():
image_id = _get_local_image_id(docker_tag=tag)
if not image_id:
raise FileNotFoundError('Unable to find docker image id matching with
{}'.format(tag))
+ # now that we've built the container, push it to our docker cache
+ push_docker_cache(registry, tag, image_id)
Review comment:
The docker image naming scheme introduced in this PR wouldn't work for
what is in master. But we don't have caching issues in master (the problem is
only in other branches since the docker containers share names but have
different dockerfiles.)
We still should be able to move master over to using ECR at some point, just
to reduce the chance of hitting dockerhub's rate limiting. For public users, we
could still publish to dockerhub but shift all CI pipelines to use images from
ECR.
----------------------------------------------------------------
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:
[email protected]
