From 3b0b77259fc280058186107a44016f5a8691465a Mon Sep 17 00:00:00 2001
From: yuvipanda <yuvipanda@gmail.com>
Date: Mon, 10 Dec 2018 11:18:01 -0800
Subject: [PATCH] Inject docker.APIClient into methods that need them

- Creating a new client with 'auto' version causes repeated
  unnecessary network requests to discover version of docker daemon.
- Testing is easier this way, since we can inject a mocked docker
  client more easily
---
 repo2docker/app.py                        | 10 ++++------
 repo2docker/buildpacks/base.py            |  4 +---
 repo2docker/buildpacks/docker.py          |  3 +--
 repo2docker/buildpacks/legacy/__init__.py |  4 ++--
 4 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/repo2docker/app.py b/repo2docker/app.py
index e90383f7..f99b7baf 100644
--- a/repo2docker/app.py
+++ b/repo2docker/app.py
@@ -697,13 +697,11 @@ class Repo2Docker(Application):
         return port
 
     def start(self):
-        """Start execution of repo2docker"""
-        # Check if r2d can connect to docker daemon
+        """Start execution of repo2docker""" # Check if r2d can connect to docker daemon
         if self.build:
             try:
-                client = docker.APIClient(version='auto',
-                                          **kwargs_from_env())
-                del client
+                api_client = docker.APIClient(version='auto',
+                                              **kwargs_from_env())
             except DockerException as e:
                 print("Docker client initialization error. Check if docker is"
                       " running on the host.")
@@ -759,7 +757,7 @@ class Repo2Docker(Application):
                     self.log.info('Using %s builder\n', bp.__class__.__name__,
                                   extra=dict(phase='building'))
 
-                    for l in picked_buildpack.build(self.output_image_spec,
+                    for l in picked_buildpack.build(api_client, self.output_image_spec,
                         self.build_memory_limit, build_args):
                         if 'stream' in l:
                             self.log.info(l['stream'],
diff --git a/repo2docker/buildpacks/base.py b/repo2docker/buildpacks/base.py
index 1cf41e26..5125c420 100644
--- a/repo2docker/buildpacks/base.py
+++ b/repo2docker/buildpacks/base.py
@@ -449,7 +449,7 @@ class BuildPack:
             appendix=self.appendix,
         )
 
-    def build(self, image_spec, memory_limit, build_args, cache_from):
+    def build(self, client, image_spec, memory_limit, build_args, cache_from):
         tarf = io.BytesIO()
         tar = tarfile.open(fileobj=tarf, mode='w')
         dockerfile_tarinfo = tarfile.TarInfo("Dockerfile")
@@ -489,8 +489,6 @@ class BuildPack:
         }
         if memory_limit:
             limits['memory'] = memory_limit
-        client = docker.APIClient(version='auto',
-                                  **docker.utils.kwargs_from_env())
         for line in client.build(
                 fileobj=tarf,
                 tag=image_spec,
diff --git a/repo2docker/buildpacks/docker.py b/repo2docker/buildpacks/docker.py
index 8644386f..5d4d83d2 100644
--- a/repo2docker/buildpacks/docker.py
+++ b/repo2docker/buildpacks/docker.py
@@ -19,7 +19,7 @@ class DockerBuildPack(BuildPack):
         with open(Dockerfile) as f:
             return f.read()
 
-    def build(self, image_spec, memory_limit, build_args, cache_from):
+    def build(self, client, image_spec, memory_limit, build_args, cache_from):
         """Build a Docker image based on the Dockerfile in the source repo."""
         limits = {
             # Always disable memory swap for building, since mostly
@@ -28,7 +28,6 @@ class DockerBuildPack(BuildPack):
         }
         if memory_limit:
             limits['memory'] = memory_limit
-        client = docker.APIClient(version='auto', **docker.utils.kwargs_from_env())
         for line in client.build(
                 path=os.getcwd(),
                 dockerfile=self.binder_path(self.dockerfile),
diff --git a/repo2docker/buildpacks/legacy/__init__.py b/repo2docker/buildpacks/legacy/__init__.py
index 33bb229d..217130c3 100644
--- a/repo2docker/buildpacks/legacy/__init__.py
+++ b/repo2docker/buildpacks/legacy/__init__.py
@@ -83,7 +83,7 @@ class LegacyBinderDockerBuildPack(DockerBuildPack):
             'legacy/python3.frozen.yml': '/tmp/python3.frozen.yml',
         }
             
-    def build(self, image_spec, memory_limit, build_args, cache_from):
+    def build(self, client, image_spec, memory_limit, build_args, cache_from):
         """Build a legacy Docker image."""
         with open(self.dockerfile, 'w') as f:
             f.write(self.render())
@@ -94,7 +94,7 @@ class LegacyBinderDockerBuildPack(DockerBuildPack):
                 env_file,
             )
             shutil.copy(src_path, env_file)
-        return super().build(image_spec, memory_limit, build_args, cache_from)
+        return super().build(client, image_spec, memory_limit, build_args, cache_from)
 
     def detect(self):
         """Check if current repo should be built with the Legacy BuildPack.