Merge pull request #3708 from mboersma/use-dict-views

mboersma · mboersma · commit 6b3d5f0643ea · 2015-05-18T11:04:44.000-06:00
More efficient iteration
diff --git a/api/models.py b/api/models.py
@@ -84,7 +84,7 @@ def validate_id_is_docker_compatible(value):
 def validate_app_structure(value):
     """Error if the dict values aren't ints >= 0."""
     try:
-        if any(int(v) < 0 for v in value.itervalues()):
+        if any(int(v) < 0 for v in value.viewvalues()):
             raise ValueError("Must be greater than or equal to zero")
     except ValueError, err:
         raise ValidationError(err)
@@ -230,7 +230,7 @@ def scale(self, user, structure):  # noqa
         release = self.release_set.latest()
         # test for available process types
         available_process_types = release.build.procfile or {}
-        for container_type in requested_structure.keys():
+        for container_type in requested_structure:
             if container_type == 'cmd':
                 continue  # allow docker cmd types in case we don't have the image source
             if container_type not in available_process_types:
@@ -242,6 +242,7 @@ def scale(self, user, structure):  # noqa
         # iterate and scale by container type (web, worker, etc)
         changed = False
         to_add, to_remove = [], []
+        # iterate on a copy of the container_type keys
         for container_type in requested_structure.keys():
             containers = list(self.container_set.filter(type=container_type).order_by('created'))
             # increment new container nums off the most recent container
@@ -613,8 +614,8 @@ def save(self, **kwargs):
         try:
             previous_build = self.app.build_set.latest()
             to_destroy = []
-            for proctype in previous_build.procfile.keys():
-                if proctype not in self.procfile.keys():
+            for proctype in previous_build.procfile:
+                if proctype not in self.procfile:
                     for c in self.app.container_set.filter(type=proctype):
                         to_destroy.append(c)
             self.app._destroy_containers(to_destroy)
@@ -665,7 +666,7 @@ def save(self, **kwargs):
                     new_data = {}
                 data.update(new_data)
                 # remove config keys if we provided a null value
-                [data.pop(k) for k, v in new_data.items() if v is None]
+                [data.pop(k) for k, v in new_data.viewitems() if v is None]
                 setattr(self, attr, data)
         except Config.DoesNotExist:
             pass
diff --git a/api/serializers.py b/api/serializers.py
@@ -141,7 +141,7 @@ class Meta:
         model = models.Config
 
     def validate_memory(self, value):
-        for k, v in value.items():
+        for k, v in value.viewitems():
             if v is None:  # use NoneType to unset a value
                 continue
             if not re.match(PROCTYPE_MATCH, k):
@@ -152,15 +152,15 @@ def validate_memory(self, value):
         return value
 
     def validate_cpu(self, value):
-        for k, v in value.items():
+        for k, v in value.viewitems():
             if v is None:  # use NoneType to unset a value
                 continue
             if not re.match(PROCTYPE_MATCH, k):
                 raise serializers.ValidationError("Process types can only contain [a-z]")
             shares = re.match(CPUSHARE_MATCH, str(v))
             if not shares:
                 raise serializers.ValidationError("CPU shares must be an integer")
-            for v in shares.groupdict().values():
+            for v in shares.groupdict().viewvalues():
                 try:
                     i = int(v)
                 except ValueError:
@@ -170,7 +170,7 @@ def validate_cpu(self, value):
         return value
 
     def validate_tags(self, value):
-        for k, v in value.items():
+        for k, v in value.viewitems():
             if v is None:  # use NoneType to unset a value
                 continue
             if not re.match(TAGKEY_MATCH, k):
diff --git a/api/tests/test_app.py b/api/tests/test_app.py
@@ -72,7 +72,7 @@ def test_response_data(self):
         response = self.client.post('/v1/apps', json.dumps(body),
                                     content_type='application/json',
                                     HTTP_AUTHORIZATION='token {}'.format(self.token))
-        for key in response.data.keys():
+        for key in response.data:
             self.assertIn(key, ['uuid', 'created', 'updated', 'id', 'owner', 'url', 'structure'])
         expected = {
             'id': 'test',
diff --git a/api/tests/test_auth.py b/api/tests/test_auth.py
@@ -42,7 +42,7 @@ def test_auth(self):
         url = '/v1/auth/register'
         response = self.client.post(url, json.dumps(submit), content_type='application/json')
         self.assertEqual(response.status_code, 201)
-        for key in response.data.keys():
+        for key in response.data:
             self.assertIn(key, ['id', 'last_login', 'is_superuser', 'username', 'first_name',
                                 'last_name', 'email', 'is_active', 'is_superuser', 'is_staff',
                                 'date_joined', 'groups', 'user_permissions'])
@@ -122,7 +122,7 @@ def test_auth_registration_admin_only_works(self):
                                     HTTP_AUTHORIZATION='token {}'.format(token))
 
         self.assertEqual(response.status_code, 201)
-        for key in response.data.keys():
+        for key in response.data:
             self.assertIn(key, ['id', 'last_login', 'is_superuser', 'username', 'first_name',
                                 'last_name', 'email', 'is_active', 'is_superuser', 'is_staff',
                                 'date_joined', 'groups', 'user_permissions'])
diff --git a/api/tests/test_build.py b/api/tests/test_build.py
@@ -95,7 +95,7 @@ def test_response_data(self):
         body = {'image': 'autotest/example'}
         response = self.client.post(url, json.dumps(body), content_type='application/json',
                                     HTTP_AUTHORIZATION='token {}'.format(self.token))
-        for key in response.data.keys():
+        for key in response.data:
             self.assertIn(key, ['uuid', 'owner', 'created', 'updated', 'app', 'dockerfile',
                                 'image', 'procfile', 'sha'])
         expected = {
diff --git a/api/tests/test_config.py b/api/tests/test_config.py
@@ -119,7 +119,7 @@ def test_response_data(self):
         body = {'values': json.dumps({'PORT': '5000'})}
         response = self.client.post(url, json.dumps(body), content_type='application/json',
                                     HTTP_AUTHORIZATION='token {}'.format(self.token))
-        for key in response.data.keys():
+        for key in response.data:
             self.assertIn(key, ['uuid', 'owner', 'created', 'updated', 'app', 'values', 'memory',
                                 'cpu', 'tags'])
         expected = {
diff --git a/api/tests/test_domain.py b/api/tests/test_domain.py
@@ -39,7 +39,7 @@ def test_response_data(self):
         response = self.client.post('/v1/apps/test/domains', json.dumps(body),
                                     content_type='application/json',
                                     HTTP_AUTHORIZATION='token {}'.format(self.token))
-        for key in response.data.keys():
+        for key in response.data:
             self.assertIn(key, ['uuid', 'owner', 'created', 'updated', 'app', 'domain'])
         expected = {
             'owner': self.user.username,
diff --git a/api/tests/test_scheduler.py b/api/tests/test_scheduler.py
@@ -232,7 +232,7 @@ def test_destroy_chaos(self):
         self.assertEqual(states, set(['error']))
         # make sure we can cleanup after enough tries
         containers = 20
-        for _ in range(100):
+        for _ in xrange(100):
             url = "/v1/apps/{app_id}/scale".format(**locals())
             body = {'web': 0}
             response = self.client.post(url, json.dumps(body), content_type='application/json',
diff --git a/api/views.py b/api/views.py
@@ -134,7 +134,7 @@ def scale(self, request, **kwargs):
         new_structure = {}
         app = self.get_object()
         try:
-            for target, count in request.data.items():
+            for target, count in request.data.viewitems():
                 new_structure[target] = int(count)
             models.validate_app_structure(new_structure)
             app.scale(request.user, new_structure)
diff --git a/registry/private.py b/registry/private.py
@@ -90,7 +90,7 @@ def _api_call(endpoint, data=None, headers={}, cookies=None, request_type='GET')
     base_headers = {'user-agent': 'docker/1.0.0'}
     r = None
     if len(headers) > 0:
-        for header, value in headers.iteritems():
+        for header, value in headers.viewitems():
             base_headers[header] = value
     if request_type == 'GET':
         r = requests.get(endpoint, headers=base_headers)
@@ -170,7 +170,7 @@ def _construct_env(env, config):
             v = config.pop(k)
         new_env.append("{}={}".format(encode(k), encode(v)))
     # add other config ENV items
-    for k, v in config.items():
+    for k, v in config.viewitems():
         new_env.append("{}={}".format(encode(k), encode(v)))
     return new_env
 
diff --git a/scheduler/fleet.py b/scheduler/fleet.py
@@ -51,7 +51,7 @@ def _request_unit(self, method, name, body=None):
         return self.conn.getresponse()
 
     def _get_unit(self, name):
-        for attempt in range(RETRIES):
+        for attempt in xrange(RETRIES):
             try:
                 resp = self._request_unit('GET', name)
                 data = resp.read()
@@ -65,7 +65,7 @@ def _get_unit(self, name):
                     raise
 
     def _put_unit(self, name, body):
-        for attempt in range(RETRIES):
+        for attempt in xrange(RETRIES):
             try:
                 resp = self._request_unit('PUT', name, body)
                 data = resp.read()
@@ -153,7 +153,7 @@ def _create_container(self, name, image, command, unit, **kwargs):
         # prepare tags only if one was provided
         tags = kwargs.get('tags', {})
         if tags:
-            tagset = ' '.join(['"{}={}"'.format(k, v) for k, v in tags.items()])
+            tagset = ' '.join(['"{}={}"'.format(k, v) for k, v in tags.viewitems()])
             unit.append({"section": "X-Fleet", "name": "MachineMetadata",
                          "value": tagset})
         # post unit to fleet
@@ -179,7 +179,7 @@ def start(self, name):
 
     def _wait_for_container_state(self, name):
         # wait for container to get scheduled
-        for _ in range(30):
+        for _ in xrange(30):
             states = self._get_state(name)
             if states and len(states.get('states', [])) == 1:
                 return states.get('states')[0]
@@ -196,15 +196,15 @@ def _wait_for_container_running(self, name):
 
     def _wait_for_job_state(self, name, state):
         # we bump to 20 minutes here to match the timeout on the router and in the app unit files
-        for _ in range(1200):
+        for _ in xrange(1200):
             if self.state(name) == state:
                 return
             time.sleep(1)
         else:
             raise RuntimeError('timeout waiting for job state: {}'.format(state))
 
     def _wait_for_destroy(self, name):
-        for _ in range(30):
+        for _ in xrange(30):
             if not self._get_state(name):
                 break
             time.sleep(1)
@@ -226,7 +226,7 @@ def destroy(self, name):
         self._wait_for_destroy(name)
 
     def _destroy_container(self, name):
-        for attempt in range(RETRIES):
+        for attempt in xrange(RETRIES):
             try:
                 self._delete_unit(name)
                 break
@@ -291,7 +291,7 @@ def _do_ssh(cmd):
                 raise RuntimeError('failed to create container')
 
             # wait for container to start
-            for _ in range(2):
+            for _ in xrange(2):
                 _rc, _output = _do_ssh('docker inspect {name}'.format(**locals()))
                 if _rc != 0:
                     raise RuntimeError('failed to inspect container')
@@ -304,7 +304,7 @@ def _do_ssh(cmd):
                 raise RuntimeError('container failed to start')
 
             # wait for container to complete
-            for _ in range(1200):
+            for _ in xrange(1200):
                 _rc, _output = _do_ssh('docker inspect {name}'.format(**locals()))
                 if _rc != 0:
                     raise RuntimeError('failed to inspect container')
diff --git a/scheduler/swarm.py b/scheduler/swarm.py
@@ -93,7 +93,7 @@ def _get_container_state(self, name):
 
     def state(self, name):
         try:
-            for _ in range(30):
+            for _ in xrange(30):
                 return self._get_container_state(name)
                 time.sleep(1)
             # FIXME (smothiki): should be able to send JobState.crashed
@@ -123,12 +123,12 @@ def _get_hostname(self, application_name):
 
     def _get_portbindings(self, image):
         dictports = self.docker_cli.inspect_image(image)['ContainerConfig']['ExposedPorts']
-        for port, mapping in dictports.items():
+        for port in dictports:
             dictports[port] = None
         return dictports
 
     def _get_ports(self, image):
         dictports = self.docker_cli.inspect_image(image)['ContainerConfig']['ExposedPorts']
-        return [int(port.split('/')[0]) for port in dictports.iterkeys()]
+        return [int(port.split('/')[0]) for port in dictports]
 
 SchedulerClient = SwarmClient
