refactor: review lint issues

Updating `make lint` command
Updating pylama settings and requirements.txt
Addressing lint issues in Access/accessrequest_helper.py file

Author: sumedha-bhatt

Access/accessrequest_helper.py

@@ -1,15 +1,21 @@
-from Access import helpers
 import logging
 import time
-from . import helpers as helper
-from Access import notifications
-
-from BrowserStackAutomation.settings import DECLINE_REASONS
-from Access.models import UserAccessMapping, User, GroupV2, AccessV2
 import datetime
 import json
 from django.db import transaction
+from Access import (
+    helpers,
+    notifications,
+)
+from Access.models import (
+    UserAccessMapping,
+    User,
+    GroupV2,
+    AccessV2,
+)
 from Access.background_task_manager import background_task
+from BrowserStackAutomation.settings import DECLINE_REASONS
+from . import helpers as helper
 
 logger = logging.getLogger(__name__)
 
@@ -50,7 +56,7 @@
                                       Pending on secondary approver"
 
 
-def requestAccessGet(request):
+def get_request_access(request):
     context = {}
     try:
         for each_tag, each_module in helpers.get_available_access_modules().items():
@@ -93,7 +99,7 @@ def requestAccessGet(request):
     return context
 
 
-def validate_approver_permissions(access_mapping, access_type, request, request_id):
+def validate_approver_permissions(access_mapping, access_type, request):
     json_response = {}
 
     access_label = access_mapping.access.access_label
@@ -117,7 +123,7 @@ def validate_approver_permissions(access_mapping, access_type, request, request_
     return json_response
 
 
-def getGrantFailedRequests(request):
+def get_grant_failed_requests(request):
     try:
         failures = UserAccessMapping.objects.filter(
             status__in=["GrantFailed"]
@@ -157,7 +163,7 @@ def get_pending_revoke_failures(request):
     return context
 
 
-def getPendingRequests(request):
+def get_pending_requests(request):
     logger.info("Pending Request call initiated")
 
     try:
@@ -174,7 +180,7 @@ def getPendingRequests(request):
         ) = get_pending_accesses_from_modules(user)
 
         duration = time.time() - start_time
-        logger.info("Time to fetch all pending requests:" + str(duration))
+        logger.info("Time to fetch all pending requests: %s " % str(duration))
 
         return context
     except Exception as e:
@@ -240,10 +246,8 @@ def get_pending_accesses_from_modules(access_user):
         process_group_requests(pending_accesses["group_requests"], group_requests)
 
         logger.info(
-            "Time to fetch pending requests of access module: "
-            + access_module_tag
-            + " - "
-            + str(time.time() - access_module_start_time)
+            "Time to fetch pending requests of access module: %s - %s " %
+            access_module_tag, str(time.time() - access_module_start_time)
         )
 
     return individual_requests, list(group_requests.values())
@@ -261,18 +265,18 @@ def process_individual_requests(
                     "club_id": club_id,
                     "userEmail": accessrequest["userEmail"],
                     "accessReason": accessrequest["accessReason"],
-                    "accessType": accessrequest["accessType"],
+                    "accessType": accessrequest["access_type"],
                     "access_tag": accessrequest["access_tag"],
                     "requested_on": accessrequest["requested_on"],
                     "sla_breached": helpers.sla_breached(accessrequest["requested_on"]),
                     "accessData": [],
                 }
-            accessData = {
+            access_data = {
                 "accessCategory": accessrequest["accessCategory"],
                 "accessMeta": accessrequest["accessMeta"],
                 "requestId": accessrequest["requestId"],
             }
-            clubbed_requests[club_id]["accessData"].append(accessData)
+            clubbed_requests[club_id]["accessData"].append(access_data)
         individual_requests.append(
             {"module_tag": access_tag, "requests": list(clubbed_requests.values())}
         )
@@ -302,15 +306,15 @@ def process_group_requests(group_pending_requests, group_requests):
                 }
             if accessrequest["access_tag"] == "other":
                 group_requests[club_id]["hasOtherRequest"] = True
-            accessData = {
+            access_data = {
                 "accessCategory": accessrequest["accessCategory"],
                 "accessMeta": accessrequest["accessMeta"],
                 "requestId": accessrequest["requestId"],
                 "accessReason": accessrequest["accessReason"],
                 "accessType": accessrequest["accessType"],
                 "access_tag": accessrequest["access_tag"],
             }
-            group_requests[club_id]["accessData"].append(accessData)
+            group_requests[club_id]["accessData"].append(access_data)
 
 
 def process_error_response(e):
@@ -646,7 +650,7 @@ def decline_individual_access(request, access_type, request_id, reason):
         return json_response
 
     json_response = validate_approver_permissions(
-        access_mapping, access_type, request, request_id
+        access_mapping, access_type, request
     )
     if "error" in json_response:
         return json_response

Access/tests/test_accessrequest_helper.py

@@ -86,7 +86,7 @@
         ),
     ],
 )
-def test_requestAccessGet(
+def test_get_request_access(
     mocker,
     contextoutput,
     getAvailableAccessModulesThrowsException,
@@ -115,19 +115,20 @@ def test_requestAccessGet(
 
     if not getAvailableAccessModulesThrowsException:
         mocker.patch(
-            "Access.helpers.get_available_access_modules", return_value={'AccModule1': accessModule}
+            "Access.helpers.get_available_access_modules",
+            return_value={"AccModule1": accessModule},
         )
     else:
         mocker.patch(
             "Access.helpers.get_available_access_modules",
-            return_value={'AccModule1': accessModule},
+            return_value={"AccModule1": accessModule},
             side_effect=Exception("getAvailableAccessModules error"),
         )
 
     request = HttpRequest()
     request.method = "GET"
     request.GET = QueryDict("accesses=access_AccModule1")
-    context = accessrequest_helper.requestAccessGet(request)
+    context = accessrequest_helper.get_request_access(request)
     if not getAvailableAccessModulesThrowsException:
         assert str(context["accesses"][0]) == str(contextoutput["accesses"][0])
     else:
@@ -230,7 +231,7 @@ def requestGet(val):
             side_effect=Exception(getUserExceptionString),
         )
 
-    context = accessrequest_helper.getGrantFailedRequests(request)
+    context = accessrequest_helper.get_grant_failed_requests(request)
     assert str(context) == expectedOutput
 
 

Access/views.py

@@ -15,10 +15,10 @@
 from .decorators import user_admin_or_ops, authentication_classes, user_with_permission
 from Access import group_helper
 from Access.accessrequest_helper import (
-    requestAccessGet,
-    getGrantFailedRequests,
+    get_request_access,
+    get_grant_failed_requests,
     get_pending_revoke_failures,
-    getPendingRequests,
+    get_pending_requests,
     create_request,
     accept_user_access_requests,
     get_decline_access_request,
@@ -86,7 +86,7 @@ def showAccessHistory(request):
 @user_admin_or_ops
 def pendingFailure(request):
     try:
-        response = getGrantFailedRequests(request)
+        response = get_grant_failed_requests(request)
         return render(request, "BSOps/failureAdminRequests.html", response)
     except Exception as e:
         logger.debug("Error in request not found OR Invalid request type")
@@ -179,7 +179,7 @@ def requestAccess(request):
         )
         return render(request, "BSOps/accessStatus.html", context)
     else:
-        context = requestAccessGet(request)
+        context = get_request_access(request)
         return render(request, "BSOps/accessRequestForm.html", context)
 
 
@@ -248,7 +248,7 @@ def add_user_to_group(request, groupName):
 @login_required
 @user_with_permission([PERMISSION_CONSTANTS["DEFAULT_APPROVER_PERMISSION"]])
 def pendingRequests(request):
-    context = getPendingRequests(request)
+    context = get_pending_requests(request)
     return render(request, "BSOps/pendingRequests.html", context)
 
 

Makefile

@@ -44,7 +44,7 @@ lint_issues:
 .PHONY: lint
 lint: lint_issues
 	@python3 -m pylama --version
-	@pylama --skip "env/*" -r lint_issues || echo "Linter run returned errors. Check lint_issues file for details." && false
+	@pylama -r lint_issues || echo "Linter run returned errors. Check lint_issues file for details." && false
 
 schema_validate:
 	@echo $(shell python3 scripts/clone_access_modules.py && python3 scripts/validator.py)

pytest.ini

@@ -2,3 +2,13 @@
 DJANGO_SETTINGS_MODULE = BrowserStackAutomation.settings
 ; # -- recommended but optional:
 ; python_files = tests.py test_*.py *_tests.py
+
+[pylama]
+skip = env/*
+linters = pycodestyle,pyflakes,mccabe,pylint,radon,eradicate,mypy,vulture
+
+[pylama:mccabe]
+max-complexity = 10
+
+[pylama:pylint]
+good_names = e

requirements.txt

@@ -107,3 +107,14 @@ django-celery-results==2.4.0
 sqlalchemy==1.4.45
 redis==4.4.0
 celery==5.2.7
+django-stubs==1.14.0
+pycodestyle==2.10.0
+pylint==2.16.2
+pylint-django==2.5.3
+pylint-plugin-utils==0.7
+mccabe==0.7.0
+radon==5.1.0
+eradicate==2.1.0
+mypy==1.0.0
+mypy-extensions==1.0.0
+vulture==2.7