Merge branch 'id-resolution-in-check-resource' of…

Merge branch 'id-resolution-in-check-resource' of https://github.com/posita/docker-py into posita-id-resolution-in-check-resource

docker/client.py

@@ -246,8 +246,6 @@ class Client(requests.Session):
     @check_resource
     def attach(self, container, stdout=True, stderr=True,
                stream=False, logs=False):
-        if isinstance(container, dict):
-            container = container.get('Id')
         params = {
             'logs': logs and 1 or 0,
             'stdout': stdout and 1 or 0,
@@ -292,9 +290,6 @@ class Client(requests.Session):
         if ws:
             return self._attach_websocket(container, params)
 
-        if isinstance(container, dict):
-            container = container.get('Id')
-
         u = self._url("/containers/{0}/attach".format(container))
         return self._get_raw_response_socket(self.post(
             u, None, params=self._attach_params(params), stream=True))
@@ -409,8 +404,6 @@ class Client(requests.Session):
     @check_resource
     def commit(self, container, repository=None, tag=None, message=None,
                author=None, conf=None):
-        if isinstance(container, dict):
-            container = container.get('Id')
         params = {
             'container': container,
             'repo': repository,
@@ -447,8 +440,6 @@ class Client(requests.Session):
 
     @check_resource
     def copy(self, container, resource):
-        if isinstance(container, dict):
-            container = container.get('Id')
         res = self._post_json(
             self._url("/containers/{0}/copy".format(container)),
             data={"Resource": resource},
@@ -493,8 +484,6 @@ class Client(requests.Session):
 
     @check_resource
     def diff(self, container):
-        if isinstance(container, dict):
-            container = container.get('Id')
         return self._result(self._get(self._url("/containers/{0}/changes".
                             format(container))), True)
 
@@ -539,8 +528,6 @@ class Client(requests.Session):
             raise errors.InvalidVersion(
                 'Privileged exec is not supported in API < 1.19'
             )
-        if isinstance(container, dict):
-            container = container.get('Id')
         if isinstance(cmd, six.string_types):
             cmd = shlex.split(str(cmd))
 
@@ -605,8 +592,6 @@ class Client(requests.Session):
 
     @check_resource
     def export(self, container):
-        if isinstance(container, dict):
-            container = container.get('Id')
         res = self._get(self._url("/containers/{0}/export".format(container)),
                         stream=True)
         self._raise_for_status(res)
@@ -744,16 +729,12 @@ class Client(requests.Session):
 
     @check_resource
     def inspect_container(self, container):
-        if isinstance(container, dict):
-            container = container.get('Id')
         return self._result(
             self._get(self._url("/containers/{0}/json".format(container))),
             True)
 
     @check_resource
     def inspect_image(self, image):
-        if isinstance(image, dict):
-            image = image.get('Id')
         return self._result(
             self._get(self._url("/images/{0}/json".format(image))),
             True
@@ -761,8 +742,6 @@ class Client(requests.Session):
 
     @check_resource
     def kill(self, container, signal=None):
-        if isinstance(container, dict):
-            container = container.get('Id')
         url = self._url("/containers/{0}/kill".format(container))
         params = {}
         if signal is not None:
@@ -810,8 +789,6 @@ class Client(requests.Session):
     @check_resource
     def logs(self, container, stdout=True, stderr=True, stream=False,
              timestamps=False, tail='all'):
-        if isinstance(container, dict):
-            container = container.get('Id')
         if utils.compare_version('1.11', self._version) >= 0:
             params = {'stderr': stderr and 1 or 0,
                       'stdout': stdout and 1 or 0,
@@ -844,8 +821,6 @@ class Client(requests.Session):
 
     @check_resource
     def pause(self, container):
-        if isinstance(container, dict):
-            container = container.get('Id')
         url = self._url('/containers/{0}/pause'.format(container))
         res = self._post(url)
         self._raise_for_status(res)
@@ -855,8 +830,6 @@ class Client(requests.Session):
 
     @check_resource
     def port(self, container, private_port):
-        if isinstance(container, dict):
-            container = container.get('Id')
         res = self._get(self._url("/containers/{0}/json".format(container)))
         self._raise_for_status(res)
         json_ = res.json()
@@ -961,8 +934,6 @@ class Client(requests.Session):
 
     @check_resource
     def remove_container(self, container, v=False, link=False, force=False):
-        if isinstance(container, dict):
-            container = container.get('Id')
         params = {'v': v, 'link': link, 'force': force}
         res = self._delete(self._url("/containers/" + container),
                            params=params)
@@ -970,8 +941,6 @@ class Client(requests.Session):
 
     @check_resource
     def remove_image(self, image, force=False, noprune=False):
-        if isinstance(image, dict):
-            image = image.get('Id')
         params = {'force': force, 'noprune': noprune}
         res = self._delete(self._url("/images/" + image), params=params)
         self._raise_for_status(res)
@@ -982,8 +951,6 @@ class Client(requests.Session):
             raise errors.InvalidVersion(
                 'rename was only introduced in API version 1.17'
             )
-        if isinstance(container, dict):
-            container = container.get('Id')
         url = self._url("/containers/{0}/rename".format(container))
         params = {'name': name}
         res = self._post(url, params=params)
@@ -991,9 +958,6 @@ class Client(requests.Session):
 
     @check_resource
     def resize(self, container, height, width):
-        if isinstance(container, dict):
-            container = container.get('Id')
-
         params = {'h': height, 'w': width}
         url = self._url("/containers/{0}/resize".format(container))
         res = self._post(url, params=params)
@@ -1001,8 +965,6 @@ class Client(requests.Session):
 
     @check_resource
     def restart(self, container, timeout=10):
-        if isinstance(container, dict):
-            container = container.get('Id')
         params = {'t': timeout}
         url = self._url("/containers/{0}/restart".format(container))
         res = self._post(url, params=params)
@@ -1067,9 +1029,6 @@ class Client(requests.Session):
             ipc_mode=ipc_mode, security_opt=security_opt, ulimits=ulimits
         )
 
-        if isinstance(container, dict):
-            container = container.get('Id')
-
         url = self._url("/containers/{0}/start".format(container))
         if not start_config:
             start_config = None
@@ -1088,15 +1047,11 @@ class Client(requests.Session):
             raise errors.InvalidVersion(
                 'Stats retrieval is not supported in API < 1.17!')
 
-        if isinstance(container, dict):
-            container = container.get('Id')
         url = self._url("/containers/{0}/stats".format(container))
         return self._stream_helper(self._get(url, stream=True), decode=decode)
 
     @check_resource
     def stop(self, container, timeout=10):
-        if isinstance(container, dict):
-            container = container.get('Id')
         params = {'t': timeout}
         url = self._url("/containers/{0}/stop".format(container))
 
@@ -1118,8 +1073,6 @@ class Client(requests.Session):
 
     @check_resource
     def top(self, container):
-        if isinstance(container, dict):
-            container = container.get('Id')
         u = self._url("/containers/{0}/top".format(container))
         return self._result(self._get(u), True)
 
@@ -1129,16 +1082,12 @@ class Client(requests.Session):
 
     @check_resource
     def unpause(self, container):
-        if isinstance(container, dict):
-            container = container.get('Id')
         url = self._url('/containers/{0}/unpause'.format(container))
         res = self._post(url)
         self._raise_for_status(res)
 
     @check_resource
     def wait(self, container, timeout=None):
-        if isinstance(container, dict):
-            container = container.get('Id')
         url = self._url("/containers/{0}/wait".format(container))
         res = self._post(url, timeout=timeout)
         self._raise_for_status(res)

docker/utils/decorators.py

@@ -12,6 +12,8 @@ def check_resource(f):
                 resource_id = kwargs.pop('container')
             elif kwargs.get('image'):
                 resource_id = kwargs.pop('image')
+        if isinstance(resource_id, dict):
+            resource_id = resource_id.get('Id')
         if not resource_id:
             raise errors.NullResource(
                 'image or container param is undefined'

tests/test.py

@@ -1894,15 +1894,16 @@ class DockerClientTest(Cleanup, base.BaseTestCase):
             timeout=DEFAULT_TIMEOUT_SECONDS
         )
 
-    def test_inspect_container_empty_id(self):
-        try:
-            self.client.inspect_container('')
-        except docker.errors.NullResource as e:
-            self.assertEqual(
-                e.args[0], 'image or container param is undefined'
-            )
-        else:
-            self.fail('Command expected NullResource exception')
+    def test_inspect_container_undefined_id(self):
+        for arg in None, '', {True: True}:
+            try:
+                self.client.inspect_container(arg)
+            except docker.errors.NullResource as e:
+                self.assertEqual(
+                    e.args[0], 'image or container param is undefined'
+                )
+            else:
+                self.fail('Command expected NullResource exception')
 
     def test_container_stats(self):
         try:
@@ -2075,15 +2076,16 @@ class DockerClientTest(Cleanup, base.BaseTestCase):
             timeout=DEFAULT_TIMEOUT_SECONDS
         )
 
-    def test_inspect_image_empty_id(self):
-        try:
-            self.client.inspect_image('')
-        except docker.errors.NullResource as e:
-            self.assertEqual(
-                e.args[0], 'image or container param is undefined'
-            )
-        else:
-            self.fail('Command expected NullResource exception')
+    def test_inspect_image_undefined_id(self):
+        for arg in None, '', {True: True}:
+            try:
+                self.client.inspect_image(arg)
+            except docker.errors.NullResource as e:
+                self.assertEqual(
+                    e.args[0], 'image or container param is undefined'
+                )
+            else:
+                self.fail('Command expected NullResource exception')
 
     def test_insert_image(self):
         try: