marcoabreu commented on a change in pull request #19654:
URL: https://github.com/apache/incubator-mxnet/pull/19654#discussion_r541166521
##########
File path: ci/build.py
##########
@@ -117,6 +123,9 @@ 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 if
DOCKER_ECR_CACHE is defined
+ if 'DOCKER_ECR_REGISTRY' in os.environ:
+ push_docker_cache(registry, tag, image_id)
Review comment:
This action breaks the security concept where only restricted jobs are
allowed to publish caches for security reasons. Also, it results in a lot of
unnecessary push queries to ECR during the PR jobs.
##########
File path: ci/build.py
##########
@@ -265,6 +277,22 @@ def load_docker_cache(tag, docker_registry) -> None:
else:
logging.info('Distributed docker cache disabled')
+def push_docker_cache(registry, tag, image_id) -> None:
Review comment:
We have a docker_cache.py file specifically for that purpose. I'd
recommend to make the appropriate changes in there instead of putting it into
this file.
----------------------------------------------------------------
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]
