Repository: docker/docker-py

Issue:
Can't pull image with stream=True
Hi,

The scenario is as follows:

Mac 10.13.6
docker version v18.06.0-ce
Python 3.6
(python package) docker==3.5.0
private docker registry (docker hub, private repo)
docker login works ✔️
docker pull $image works ✔️
however, pulling via the docker python api fails when using the parameter `stream=True`:
In terminal window I run :
```
python
>>> client = docker.DockerClient(base_url='unix://var/run/docker.sock')
>>> client.login(username='XXXX', password='XXXX', registry='https://index.docker.io/v1/')
>>> client.images.pull('docker.io/vdmtl/portail-datamigration-worker-gcs-lib:latest', stream=True)

Traceback (most recent call last):
  File "/Users/XXXX/apps/open-data-etl/venv/lib/python3.6/site-packages/docker/api/client.py", line 229, in _raise_for_status
    response.raise_for_status()
  File "/Users/XXX/apps/open-data-etl/venv/lib/python3.6/site-packages/requests/models.py", line 937, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 404 Client Error: Not Found for url: http+docker://localhost/v1.35/images/docker.io/vdmtl/portail-datamigration-worker-gcs-lib:latest/json

>>> client.images.pull('docker.io/vdmtl/portail-datamigration-worker-gcs-lib:latest')
<Image: 'vdmtl/portail-datamigration-worker-gcs-lib:latest'>
```
As you can see removing `stream=True` is necessary in order to download my image.





Discussion:

Patch:
diff --git a/docker/api/image.py b/docker/api/image.py
index a9f801e9..d3fed5c0 100644
--- a/docker/api/image.py
+++ b/docker/api/image.py
@@ -334,7 +334,8 @@ class ImageApiMixin(object):
         Args:
             repository (str): The repository to pull
             tag (str): The tag to pull
-            stream (bool): Stream the output as a generator
+            stream (bool): Stream the output as a generator. Make sure to
+                consume the generator, otherwise pull might get cancelled.
             auth_config (dict): Override the credentials that
                 :py:meth:`~docker.api.daemon.DaemonApiMixin.login` has set for
                 this request. ``auth_config`` should contain the ``username``
diff --git a/docker/models/images.py b/docker/models/images.py
index 4578c0bd..30e86f10 100644
--- a/docker/models/images.py
+++ b/docker/models/images.py
@@ -1,5 +1,6 @@
 import itertools
 import re
+import warnings
 
 import six
 
@@ -425,7 +426,21 @@ class ImageCollection(Collection):
         if not tag:
             repository, tag = parse_repository_tag(repository)
 
-        self.client.api.pull(repository, tag=tag, **kwargs)
+        if 'stream' in kwargs:
+            warnings.warn(
+                '`stream` is not a valid parameter for this method'
+                ' and will be overridden'
+            )
+            del kwargs['stream']
+
+        pull_log = self.client.api.pull(
+            repository, tag=tag, stream=True, **kwargs
+        )
+        for _ in pull_log:
+            # We don't do anything with the logs, but we need
+            # to keep the connection alive and wait for the image
+            # to be pulled.
+            pass
         if tag:
             return self.get('{0}{2}{1}'.format(
                 repository, tag, '@' if tag.startswith('sha256:') else ':'


Test patch:
diff --git a/tests/unit/models_containers_test.py b/tests/unit/models_containers_test.py
index 22dd2410..39e409e4 100644
--- a/tests/unit/models_containers_test.py
+++ b/tests/unit/models_containers_test.py
@@ -232,7 +232,9 @@ class ContainerCollectionTest(unittest.TestCase):
         container = client.containers.run('alpine', 'sleep 300', detach=True)
 
         assert container.id == FAKE_CONTAINER_ID
-        client.api.pull.assert_called_with('alpine', platform=None, tag=None)
+        client.api.pull.assert_called_with(
+            'alpine', platform=None, tag=None, stream=True
+        )
 
     def test_run_with_error(self):
         client = make_fake_client()
diff --git a/tests/unit/models_images_test.py b/tests/unit/models_images_test.py
index 67832795..fd894ab7 100644
--- a/tests/unit/models_images_test.py
+++ b/tests/unit/models_images_test.py
@@ -1,6 +1,8 @@
+import unittest
+import warnings
+
 from docker.constants import DEFAULT_DATA_CHUNK_SIZE
 from docker.models.images import Image
-import unittest
 
 from .fake_api import FAKE_IMAGE_ID
 from .fake_api_client import make_fake_client
@@ -43,7 +45,9 @@ class ImageCollectionTest(unittest.TestCase):
     def test_pull(self):
         client = make_fake_client()
         image = client.images.pull('test_image:latest')
-        client.api.pull.assert_called_with('test_image', tag='latest')
+        client.api.pull.assert_called_with(
+            'test_image', tag='latest', stream=True
+        )
         client.api.inspect_image.assert_called_with('test_image:latest')
         assert isinstance(image, Image)
         assert image.id == FAKE_IMAGE_ID
@@ -51,7 +55,9 @@ class ImageCollectionTest(unittest.TestCase):
     def test_pull_multiple(self):
         client = make_fake_client()
         images = client.images.pull('test_image')
-        client.api.pull.assert_called_with('test_image', tag=None)
+        client.api.pull.assert_called_with(
+            'test_image', tag=None, stream=True
+        )
         client.api.images.assert_called_with(
             all=False, name='test_image', filters=None
         )
@@ -61,6 +67,16 @@ class ImageCollectionTest(unittest.TestCase):
         assert isinstance(image, Image)
         assert image.id == FAKE_IMAGE_ID
 
+    def test_pull_with_stream_param(self):
+        client = make_fake_client()
+        with warnings.catch_warnings(record=True) as w:
+            client.images.pull('test_image', stream=True)
+
+        assert len(w) == 1
+        assert str(w[0].message).startswith(
+            '`stream` is not a valid parameter'
+        )
+
     def test_push(self):
         client = make_fake_client()
         client.images.push('foobar', insecure_registry=True)