Code cleanup and version guards

Signed-off-by: Joffrey F <joffrey@docker.com>

docker/api/service.py

@@ -73,6 +73,11 @@ def _check_api_features(version, task_template, update_config, endpoint_spec):
                 if container_spec.get('Isolation') is not None:
                     raise_version_error('ContainerSpec.isolation', '1.35')
 
+        if task_template.get('Resources'):
+            if utils.version_lt(version, '1.35'):
+                if task_template['Resources'].get('GenericResources'):
+                    raise_version_error('Resources.generic_resources', '1.35')
+
 
 def _merge_task_template(current, override):
     merged = current.copy()

docker/types/services.py

@@ -306,11 +306,10 @@ class Resources(dict):
         mem_limit (int): Memory limit in Bytes.
         cpu_reservation (int): CPU reservation in units of 10^9 CPU shares.
         mem_reservation (int): Memory reservation in Bytes.
-        generic_resources (dict:`list`): List of node level generic
+        generic_resources (dict or :py:class:`list`): Node level generic
-          resources, for example a GPU, in the form of
+          resources, for example a GPU, using the following format:
-          ``{ ResourceSpec: { 'Kind': kind, 'Value': value }}``, where
+          ``{ resource_name: resource_value }``. Alternatively, a list of
-          ResourceSpec is one of 'DiscreteResourceSpec' or 'NamedResourceSpec'
+          of resource specifications as defined by the Engine API.
-          or in the form of ``{ resource_name: resource_value }``.
     """
     def __init__(self, cpu_limit=None, mem_limit=None, cpu_reservation=None,
                  mem_reservation=None, generic_resources=None):
@@ -325,39 +324,41 @@ class Resources(dict):
         if mem_reservation is not None:
             reservation['MemoryBytes'] = mem_reservation
         if generic_resources is not None:
-            # if isinstance(generic_resources, list):
+            reservation['GenericResources'] = (
-            #     reservation['GenericResources'] = generic_resources
+                _convert_generic_resources_dict(generic_resources)
-            if isinstance(generic_resources, (list, tuple)):
+            )
-                reservation['GenericResources'] = list(generic_resources)
-            elif isinstance(generic_resources, dict):
-                resources = []
-                for kind, value in six.iteritems(generic_resources):
-                    resource_type = None
-                    if isinstance(value, int):
-                        resource_type = 'DiscreteResourceSpec'
-                    elif isinstance(value, str):
-                        resource_type = 'NamedResourceSpec'
-                    else:
-                        raise errors.InvalidArgument(
-                            'Unsupported generic resource reservation '
-                            'type: {}'.format({kind: value})
-                        )
-                    resources.append({
-                        resource_type: {'Kind': kind, 'Value': value}
-                    })
-                reservation['GenericResources'] = resources
-            else:
-                raise errors.InvalidArgument(
-                    'Unsupported generic resources '
-                    'type: {}'.format(generic_resources)
-                )
-
         if limits:
             self['Limits'] = limits
         if reservation:
             self['Reservations'] = reservation
 
 
+def _convert_generic_resources_dict(generic_resources):
+    if isinstance(generic_resources, list):
+        return generic_resources
+    if not isinstance(generic_resources, dict):
+        raise errors.InvalidArgument(
+            'generic_resources must be a dict or a list'
+            ' (found {})'.format(type(generic_resources))
+        )
+    resources = []
+    for kind, value in six.iteritems(generic_resources):
+        resource_type = None
+        if isinstance(value, int):
+            resource_type = 'DiscreteResourceSpec'
+        elif isinstance(value, str):
+            resource_type = 'NamedResourceSpec'
+        else:
+            raise errors.InvalidArgument(
+                'Unsupported generic resource reservation '
+                'type: {}'.format({kind: value})
+            )
+        resources.append({
+            resource_type: {'Kind': kind, 'Value': value}
+        })
+    return resources
+
+
 class UpdateConfig(dict):
     """
 

tests/integration/api_service_test.py

@@ -4,6 +4,7 @@ import random
 import time
 
 import docker
+import pytest
 import six
 
 from ..helpers import (
@@ -225,6 +226,7 @@ class ServiceTest(BaseAPIIntegrationTest):
         svc_id = self.client.create_service(task_tmpl, name=name)
         return resources, self.client.inspect_service(svc_id)
 
+    @requires_api_version('1.35')
     def test_create_service_with_generic_resources(self):
         successful = [{
             'input': [
@@ -256,12 +258,10 @@ class ServiceTest(BaseAPIIntegrationTest):
             expected = test.get('expected', test['input'])
             assert sorted(actual, key=_key) == sorted(expected, key=_key)
 
+    def test_create_service_with_invalid_generic_resources(self):
         for test_input in ['1', 1.0, lambda: '1', {1, 2}]:
-            try:
+            with pytest.raises(docker.errors.InvalidArgument):
                 self._create_service_with_generic_resources(test_input)
-                self.fail('Should fail: {}'.format(test_input))
-            except docker.errors.InvalidArgument:
-                pass
 
     def test_create_service_with_update_config(self):
         container_spec = docker.types.ContainerSpec(BUSYBOX, ['true'])