Fixed tests and code quality

Crystal-SDS · Feb 8, 2017 · 4f3d01f · 4f3d01f
1 parent 0bacbea
commit 4f3d01f
Show file tree

Hide file tree

Showing 18 changed files with 54 additions and 101 deletions.
diff --git a/api/api/middleware.py b/api/api/middleware.py
@@ -11,7 +11,7 @@
 valid_tokens = dict()
 
 
-class CrystalMiddleware:
+class CrystalMiddleware(object):
     def __init__(self):
         pass
 

diff --git a/api/api/settings.py b/api/api/settings.py
@@ -47,7 +47,6 @@
     'bootstrap3',
     'rest_framework',
     'filters',
-    'bw',
     'swift',
     'registry'
 )

diff --git a/api/bw/__init__.py b/api/bw/__init__.py
diff --git a/api/bw/admin.py b/api/bw/admin.py
diff --git a/api/bw/models.py b/api/bw/models.py
diff --git a/api/bw/tests.py b/api/bw/tests.py
diff --git a/api/bw/urls.py b/api/bw/urls.py
diff --git a/api/bw/views.py b/api/bw/views.py
diff --git a/api/filters/views.py b/api/filters/views.py
@@ -103,26 +103,26 @@ def storlet_detail(request, storlet_id):
         return JSONResponse('Object does not exist!', status=status.HTTP_404_NOT_FOUND)
 
     if request.method == 'GET':
-        filter = r.hgetall("filter:" + str(storlet_id))
+        my_filter = r.hgetall("filter:" + str(storlet_id))
 
-        to_json_bools(filter, 'has_reverse', 'is_pre_get', 'is_post_get', 'is_pre_put', 'is_post_put', 'enabled')
-        return JSONResponse(filter, status=status.HTTP_200_OK)
+        to_json_bools(my_filter, 'has_reverse', 'is_pre_get', 'is_post_get', 'is_pre_put', 'is_post_put', 'enabled')
+        return JSONResponse(my_filter, status=status.HTTP_200_OK)
 
     elif request.method == 'PUT':
         try:
             data = JSONParser().parse(request)
         except ParseError:
             return JSONResponse("Invalid format or empty request", status=status.HTTP_400_BAD_REQUEST)
 
-        filter = r.hgetall("filter:" + str(storlet_id))
+        my_filter = r.hgetall("filter:" + str(storlet_id))
 
-        if (((filter['filter_type'] == 'storlet' or filter['filter_type'] == 'native') and not check_keys(data.keys(), FILTER_KEYS[3:-1])) or
-                ((filter['filter_type'] == 'global') and not check_keys(data.keys(), GLOBAL_FILTER_KEYS[3:-1]))):
+        if (((my_filter['filter_type'] == 'storlet' or my_filter['filter_type'] == 'native') and not check_keys(data.keys(), FILTER_KEYS[3:-1])) or
+                ((my_filter['filter_type'] == 'global') and not check_keys(data.keys(), GLOBAL_FILTER_KEYS[3:-1]))):
             return JSONResponse("Invalid parameters in request", status=status.HTTP_400_BAD_REQUEST)
 
         try:
             r.hmset('filter:' + str(storlet_id), data)
-            if filter['filter_type'] == 'global':
+            if my_filter['filter_type'] == 'global':
                 if data['enabled'] is True or data['enabled'] == 'True' or data['enabled'] == 'true':
                     to_json_bools(data, 'has_reverse', 'is_pre_get', 'is_post_get', 'is_pre_put', 'is_post_put', 'enabled')
                     data['filter_type'] = 'global'  # Adding filter type
@@ -142,9 +142,9 @@ def storlet_detail(request, storlet_id):
                 if dsl_filter_id == storlet_id:
                     return JSONResponse('Unable to delete filter, is in use by the Registry DSL.', status=status.HTTP_403_FORBIDDEN)
 
-            filter = r.hgetall("filter:" + str(storlet_id))
+            my_filter = r.hgetall("filter:" + str(storlet_id))
             r.delete("filter:" + str(storlet_id))
-            if filter['filter_type'] == 'global':
+            if my_filter['filter_type'] == 'global':
                 r.hdel("global_filters", str(storlet_id))
             return JSONResponse('Filter has been deleted', status=status.HTTP_204_NO_CONTENT)
         except DataError:

diff --git a/api/registry/dsl_parser.py b/api/registry/dsl_parser.py
@@ -1,5 +1,5 @@
 from pyparsing import Word, Suppress, alphas, Literal, Group, Combine, opAssoc, alphanums
-from pyparsing import Regex, operatorPrecedence, oneOf, ZeroOrMore, nums, Optional, delimitedList
+from pyparsing import Regex, operatorPrecedence, oneOf, nums, Optional, delimitedList
 from django.conf import settings
 import redis
 

diff --git a/api/registry/dynamic_policies/rules/rule.py b/api/registry/dynamic_policies/rules/rule.py
@@ -110,7 +110,7 @@ def _add_metric(self, workload_name):
         """
         if workload_name not in self.observers_values.keys():
             # Trying the new PyActive version. New lookup function.
-            logger.info("Rule, Workload name:", workload_name)
+            logger.info("Rule, Workload name: " + workload_name)
             observer = self.host.lookup(workload_name)
             logger.info('Rule, Observer: ' + str(observer.get_id()) + " " + observer)
             observer.attach(self.proxy)

diff --git a/api/registry/dynamic_policies/rules/sample_bw_controllers/simple_proportional_bandwidth.py b/api/registry/dynamic_policies/rules/sample_bw_controllers/simple_proportional_bandwidth.py
@@ -40,7 +40,7 @@ def compute_algorithm(self, info):
                     for ip in assign[account][policy][device]['ips']:
                         try:
                             bw_a[account][ip+"-"+policy+"-"+device] = float(bw[account][policy])/assign[account][policy][device]['requests']
-                        except Exception as e:
+                        except Exception:
                             # TODO: NO CONTINUE
                             print "Error calculating bandwidth in simple_proportional_bandwidth rule: " + str(sys.exc_info()[0])
 

diff --git a/api/registry/tests_dynamic_policies.py b/api/registry/tests_dynamic_policies.py
@@ -47,7 +47,7 @@ def tearDown(self):
 
     def test_get_target_ok(self):
         self.setup_dsl_parser_data()
-        has_condition_list, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
+        _, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
         target = '4f0279da74ef4584a29dc72c835fe2c9'
         host = None
         rule = Rule(parsed_rule, parsed_rule.action_list[0], target, host)
@@ -56,7 +56,7 @@ def test_get_target_ok(self):
     @mock.patch('registry.dynamic_policies.rules.rule.Rule._do_action')
     def test_action_is_not_triggered(self, mock_do_action):
         self.setup_dsl_parser_data()
-        has_condition_list, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
+        _, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
         target = '4f0279da74ef4584a29dc72c835fe2c9'
         host = None
         rule = Rule(parsed_rule, parsed_rule.action_list[0], target, host)
@@ -66,7 +66,7 @@ def test_action_is_not_triggered(self, mock_do_action):
     @mock.patch('registry.dynamic_policies.rules.rule.Rule._do_action')
     def test_action_is_triggered(self, mock_do_action):
         self.setup_dsl_parser_data()
-        has_condition_list, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
+        _, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
         target = '4f0279da74ef4584a29dc72c835fe2c9'
         host = None
         rule = Rule(parsed_rule, parsed_rule.action_list[0], target, host)
@@ -81,7 +81,7 @@ def test_action_set_is_triggered_deploy_200(self, mock_admin_login, mock_redis_h
                                            'identifier': '1',
                                            'valid_parameters': '{"cparam1": "integer", "cparam2": "integer", "cparam3": "integer"}'}
         self.setup_dsl_parser_data()
-        has_condition_list, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
+        _, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
         target = '4f0279da74ef4584a29dc72c835fe2c9'
         host = None
         rule = Rule(parsed_rule, parsed_rule.action_list[0], target, host)
@@ -100,7 +100,7 @@ def test_action_set_is_triggered_deploy_400(self, mock_admin_login, mock_redis_h
                                            'identifier': '1',
                                            'valid_parameters': '{"cparam1": "integer", "cparam2": "integer", "cparam3": "integer"}'}
         self.setup_dsl_parser_data()
-        has_condition_list, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
+        _, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
         target = '4f0279da74ef4584a29dc72c835fe2c9'
         host = None
         rule = Rule(parsed_rule, parsed_rule.action_list[0], target, host)
@@ -118,7 +118,7 @@ def test_action_delete_is_triggered_undeploy_200(self, mock_stop_actor, mock_adm
                                            'identifier': '1',
                                            'valid_parameters': '{"cparam1": "integer", "cparam2": "integer", "cparam3": "integer"}'}
         self.setup_dsl_parser_data()
-        has_condition_list, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
+        _, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression')
         target = '4f0279da74ef4584a29dc72c835fe2c9'
         host = None
         action = parsed_rule.action_list[0]
@@ -137,7 +137,7 @@ def test_action_delete_is_triggered_undeploy_200(self, mock_stop_actor, mock_adm
     @mock.patch('registry.dynamic_policies.rules.rule_transient.TransientRule.do_action')
     def test_transient_action_is_triggered(self, mock_do_action):
         self.setup_dsl_parser_data()
-        has_condition_list, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression TRANSIENT')
+        _, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression TRANSIENT')
         target = '4f0279da74ef4584a29dc72c835fe2c9'
         host = None
         rule = TransientRule(parsed_rule, parsed_rule.action_list[0], target, host)
@@ -153,7 +153,7 @@ def test_transient_action_set_is_triggered_200(self, mock_requests_delete, mock_
                                            'identifier': '1',
                                            'valid_parameters': '{"cparam1": "integer", "cparam2": "integer", "cparam3": "integer"}'}
         self.setup_dsl_parser_data()
-        has_condition_list, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression TRANSIENT')
+        _, parsed_rule = parse('FOR TENANT:4f0279da74ef4584a29dc72c835fe2c9 WHEN metric1 > 5 DO SET compression TRANSIENT')
         target = '4f0279da74ef4584a29dc72c835fe2c9'
         host = None
         rule = TransientRule(parsed_rule, parsed_rule.action_list[0], target, host)
@@ -277,7 +277,7 @@ def test_min_tenant_slo_global_spare_bw_share(self, mock_pika):
     #
 
     @mock.patch('registry.dynamic_policies.rules.base_global_controller.pika')
-    def test_min_tenant_slo_global_spare_bw_share(self, mock_pika):
+    def test_min_tenant_slo_global_spare_bw_share_v2(self, mock_pika):
         self.r.set('SLO:bandwidth:put_bw:AUTH_1234567890abcdef#0', 50)
 
         smin = MinTenantSLOGlobalSpareBWShareV2('the_name', 'PUT')

diff --git a/api/registry/views.py b/api/registry/views.py
@@ -3,7 +3,6 @@
 import mimetypes
 import os
 import re
-import sys
 from operator import itemgetter
 from eventlet import sleep
 

diff --git a/api/swift/sds_project.py b/api/swift/sds_project.py
@@ -2,6 +2,7 @@
 from swiftclient import client
 import subprocess
 
+
 def add_new_sds_project(tenant_name):
     admin_user = settings.MANAGEMENT_ADMIN_USERNAME
     admin_password = settings.MANAGEMENT_ADMIN_PASSWORD
@@ -11,17 +12,17 @@ def add_new_sds_project(tenant_name):
 
     print "Creating new SDS project"
     print 'sudo python '+bin_dir+'/add_new_tenant.py '+tenant_name+' '+admin_user+' '+admin_password
-    new_project = subprocess.Popen(['sudo','python', bin_dir+'/add_new_tenant.py', tenant_name, admin_user, admin_password])
+    new_project = subprocess.Popen(['sudo', 'python', bin_dir+'/add_new_tenant.py', tenant_name, admin_user, admin_password])
     new_project.communicate()
 
     print "Deploying docker images"
     print 'sudo python '+bin_dir+'/deploy_image.py '+tenant_name+' '+tar_file+' '+docker_image
-    deploy_image = subprocess.Popen(['sudo','python', bin_dir+'/deploy_image.py', tenant_name, tar_file, docker_image])
+    deploy_image = subprocess.Popen(['sudo', 'python', bin_dir+'/deploy_image.py', tenant_name, tar_file, docker_image])
     deploy_image.communicate()
 
     print "Setting container permissions for admin user"
-    headers = {'X-Container-Read':'*:'+admin_user,'X-Container-Write':'*:'+admin_user}
+    headers = {'X-Container-Read': '*:' + admin_user, 'X-Container-Write': '*:' + admin_user}
     os_options = {'tenant_name': tenant_name}
-    url, token = client.get_auth(settings.KEYSTONE_ADMIN_URL, admin_user, admin_password, os_options = os_options, auth_version="2.0")
+    url, token = client.get_auth(settings.KEYSTONE_ADMIN_URL, admin_user, admin_password, os_options=os_options, auth_version="2.0")
     client.post_container(url, token, "storlet", headers)
-    client.post_container(url, token, "dependency", headers)
+    client.post_container(url, token, "dependency", headers)
diff --git a/api/swift/storage_policies.py → api/swift/storage_policies_utils.py b/api/swift/storage_policies.py → api/swift/storage_policies_utils.py
@@ -7,31 +7,31 @@
 # TODO: Define the parameters.
 def create(data):
     # get_hosts_object()
-    
+
     print 'lendata', len(data)
     print 'data', data
-    
+
     if len(data) == 6:
         p = subprocess.Popen(['ansible-playbook',
                               '-s',
-                              '-i', settings.ANSIBLE_DIR+'/playbook/swift_cluster_nodes',
-                              settings.ANSIBLE_DIR+'/playbook/swift_create_new_storage_policy.yml',
+                              '-i', settings.ANSIBLE_DIR + '/playbook/swift_cluster_nodes',
+                              settings.ANSIBLE_DIR + '/playbook/swift_create_new_storage_policy.yml',
                               '-e', 'policy_id=' + str(data["policy_id"]),
                               '-e', 'name=' + data["name"],
                               '-e', 'partitions=' + str(data["partitions"]),
                               '-e', 'replicas=' + str(data["replicas"]),
                               '-e', 'time=' + data["time"],
                               '-e', "storage_node=" + data["storage_node"]],
-                              env={"ANSIBLE_HOST_KEY_CHECKING": "False"},
-                              stdout=subprocess.PIPE,
-                              stderr=subprocess.PIPE)
+                             env={"ANSIBLE_HOST_KEY_CHECKING": "False"},
+                             stdout=subprocess.PIPE,
+                             stderr=subprocess.PIPE)
         _monitor_playbook_execution(p)
-        
+
     else:
         p = subprocess.Popen(['ansible-playbook',
                               '-s',
-                              '-i', settings.ANSIBLE_DIR+'/playbook/swift_cluster_nodes',
-                              settings.ANSIBLE_DIR+'/playbook/swift_create_new_storage_policy.yml',
+                              '-i', settings.ANSIBLE_DIR + '/playbook/swift_cluster_nodes',
+                              settings.ANSIBLE_DIR + '/playbook/swift_create_new_storage_policy.yml',
                               '-e', 'policy_id=' + str(data["policy_id"]),
                               '-e', 'name=' + data["name"],
                               '-e', 'partitions=' + str(data["partitions"]),
@@ -42,19 +42,19 @@ def create(data):
                               '-e', "ec_num_data_fragments=" + str(data["ec_num_data_fragments"]),
                               '-e', "ec_num_parity_fragments=" + str(data["ec_num_parity_fragments"]),
                               '-e', "ec_object_segment_size=" + str(data["ec_object_segment_size"])],
-                              env={"ANSIBLE_HOST_KEY_CHECKING": "False"},
-                              stdout=subprocess.PIPE,
-                              stderr=subprocess.PIPE)
+                             env={"ANSIBLE_HOST_KEY_CHECKING": "False"},
+                             stdout=subprocess.PIPE,
+                             stderr=subprocess.PIPE)
         _monitor_playbook_execution(p)
 
     p = subprocess.Popen(['ansible-playbook', '-vvv',
                           '-s',
-                          '-i', settings.ANSIBLE_DIR+'/playbook/swift_cluster_nodes',
-                          settings.ANSIBLE_DIR+'/playbook/distribute_ring_to_storage_nodes.yml',
+                          '-i', settings.ANSIBLE_DIR + '/playbook/swift_cluster_nodes',
+                          settings.ANSIBLE_DIR + '/playbook/distribute_ring_to_storage_nodes.yml',
                           '-e', 'policy_id=' + data["policy_id"]],
-                          env={"ANSIBLE_HOST_KEY_CHECKING": "False"},
-                          stdout=subprocess.PIPE,
-                          stderr=subprocess.PIPE)
+                         env={"ANSIBLE_HOST_KEY_CHECKING": "False"},
+                         stdout=subprocess.PIPE,
+                         stderr=subprocess.PIPE)
     _monitor_playbook_execution(p)
 
 

diff --git a/api/swift/views.py b/api/swift/views.py
@@ -13,7 +13,7 @@
 from operator import itemgetter
 
 import sds_project
-import storage_policies
+import storage_policies_utils
 from api.common_utils import JSONResponse, get_redis_connection, get_token_connection
 from api.exceptions import FileSynchronizationException
 
@@ -79,7 +79,7 @@ def storage_policies(request):
                 storage_nodes_list.extend([k, v])
             data["storage_node"] = ','.join(map(str, storage_nodes_list))
             try:
-                storage_policies.create(data)
+                storage_policies_utils.create(data)
             except Exception as e:
                 return JSONResponse('Error creating the Storage Policy: ' + e, status=status.HTTP_500_INTERNAL_SERVER_ERROR)
 
@@ -141,7 +141,7 @@ def sort_list(request):
 
 
 @csrf_exempt
-def sort_detail(request, id):
+def sort_detail(request, sort_id):
     """
     Retrieve, update or delete a Proxy Sorting.
     """
@@ -152,21 +152,21 @@ def sort_detail(request, id):
         return JSONResponse('Error connecting with DB', status=status.HTTP_500_INTERNAL_SERVER_ERROR)
 
     if request.method == 'GET':
-        proxy_sorting = r.hgetall("proxy_sorting:" + str(id))
+        proxy_sorting = r.hgetall("proxy_sorting:" + str(sort_id))
         return JSONResponse(proxy_sorting, status=status.HTTP_200_OK)
 
     elif request.method == 'PUT':
         try:
             data = JSONParser().parse(request)
-            r.hmset('proxy_sorting:' + str(id), data)
+            r.hmset('proxy_sorting:' + str(sort_id), data)
             return JSONResponse("Data updated", status=status.HTTP_201_CREATED)
         except redis.exceptions.DataError:
             return JSONResponse("Error updating data", status=status.HTTP_400_BAD_REQUEST)
         except ParseError:
             return JSONResponse("Invalid format or empty request", status=status.HTTP_400_BAD_REQUEST)
 
     elif request.method == 'DELETE':
-        r.delete("proxy_sorting:" + str(id))
+        r.delete("proxy_sorting:" + str(sort_id))
         return JSONResponse('Proxy sorting has been deleted', status=status.HTTP_204_NO_CONTENT)
     return JSONResponse('Method ' + str(request.method) + ' not allowed.', status=status.HTTP_405_METHOD_NOT_ALLOWED)
 

diff --git a/test-requirements.txt b/test-requirements.txt
@@ -4,4 +4,5 @@
 
 mock>=2.0 # BSD
 coveralls>=1.1
-httmock
+httmock
+eventlet