QA inspections

keystone_api/apps/allocations/tasks.py

@@ -78,7 +78,7 @@ def update_limit_for_account(account: ResearchGroup, cluster: Cluster) -> None:
     historical_usage = current_limit - active_sus - closing_sus
     if historical_usage < 0:
         log.warning(f"Negative Historical usage found for {account.name} on {cluster.name}:\n"
-                    f"historical: {historical_usage} = current limit: {current_limit} - active: {active_sus} - closing: {closing_sus}\n"
+                    f"historical: {historical_usage}, current: {current_limit}, active: {active_sus}, closing: {closing_sus}\n"
                     f"Assuming zero...")
         historical_usage = 0
 
@@ -92,32 +92,33 @@ def update_limit_for_account(account: ResearchGroup, cluster: Cluster) -> None:
         current_usage = historical_usage
 
     closing_summary = (f"Summary of closing allocations:\n"
-                       f"    Current Usage before closing: {current_usage}\n")
+                       f"> Current Usage before closing: {current_usage}\n")
     for allocation in closing_query.all():
         allocation.final = min(current_usage, allocation.awarded)
-        closing_summary += f"    Allocation {allocation.id}: {current_usage} - {allocation.final} -> {current_usage - allocation.final}\n"
+        closing_summary += f"> Allocation {allocation.id}: {current_usage} - {allocation.final} -> {current_usage - allocation.final}\n"
         current_usage -= allocation.final
         allocation.save()
-    closing_summary += f"    Current Usage after closing: {current_usage}"
+    closing_summary += f"> Current Usage after closing: {current_usage}"
 
     # This shouldn't happen but if it does somehow, create a warning so an admin will notice
     if current_usage > active_sus:
         log.warning(f"The current usage is somehow higher than the limit for {account.name}!")
 
     # Set the new account usage limit using the updated historical usage after closing any expired allocations
-    updated_historical_usage = approved_query.filter(request__expire__lte=date.today()).aggregate(Sum("final"))['final__sum'] or 0
+    expired_requests = approved_query.filter(request__expire__lte=date.today())
+    updated_historical_usage = expired_requests.aggregate(Sum("final"))['final__sum'] or 0
 
     updated_limit = updated_historical_usage + active_sus
     set_cluster_limit(account.name, cluster.name, updated_limit)
 
     # Log summary of changes during limits update for this Slurm account on this cluster
     log.debug(f"Summary of limits update for {account.name} on {cluster.name}:\n"
-              f"    Approved allocations found: {len(approved_query)}\n"
-              f"    Service units from {len(active_query)} active allocations: {active_sus}\n"
-              f"    Service units from {len(closing_query)} closing allocations: {closing_sus}\n"
-              f"    {closing_summary}"
-              f"    historical usage change: {historical_usage} -> {updated_historical_usage}\n"
-              f"    limit change: {current_limit} -> {updated_limit}")
+              f"> Approved allocations found: {len(approved_query)}\n"
+              f"> Service units from {len(active_query)} active allocations: {active_sus}\n"
+              f"> Service units from {len(closing_query)} closing allocations: {closing_sus}\n"
+              f"> {closing_summary}"
+              f"> historical usage change: {historical_usage} -> {updated_historical_usage}\n"
+              f"> limit change: {current_limit} -> {updated_limit}")
 
 
 def send_expiry_notification_for_request(user: User, request: AllocationRequest) -> None:
@@ -145,7 +146,7 @@ def send_expiry_notification_for_request(user: User, request: AllocationRequest)
     )
 
     # Exit early if we have not hit a threshold yet
-    log.debug(f'Request #{request.id} expires in {days_until_expire} days with next threshold at {next_threshold} days.')
+    log.debug(f'Request #{request.id} expires in {days_until_expire} days. Next threshold at {next_threshold} days.')
     if next_threshold is None:
         return
 

keystone_api/apps/health/views.py

@@ -7,7 +7,7 @@
 from django.http import HttpResponse, JsonResponse
 from django.utils.decorators import method_decorator
 from django.views.decorators.cache import cache_page
-from drf_spectacular.utils import extend_schema, inline_serializer, OpenApiExample
+from drf_spectacular.utils import extend_schema, inline_serializer
 from health_check.mixins import CheckMixin
 from rest_framework import serializers
 from rest_framework.generics import GenericAPIView

keystone_api/apps/logging/admin.py

@@ -31,12 +31,18 @@ class ReadOnlyModelAdminMixin:
     """Mixin class for creating model admins with read only permissions."""
 
     def has_change_permission(self, request, obj=None) -> False:
+        """Disable permissions for modifying records."""
+
         return False
 
     def has_add_permission(self, request, obj=None) -> False:
+        """Disable permissions for creating new records."""
+
         return False
 
     def has_delete_permission(self, request, obj=None) -> False:
+        """Disable permissions for deleting records."""
+
         return False
 
 

keystone_api/apps/logging/tests/tasks/test_rotate_log_files.py

@@ -4,7 +4,7 @@
 
 from django.test import override_settings, TestCase
 
-from apps.logging.models import *
+from apps.logging.models import AppLog, RequestLog
 from apps.logging.tasks import rotate_log_files
 
 

keystone_api/apps/users/management/commands/ldap_update.py

@@ -23,7 +23,7 @@ def add_arguments(self, parser: ArgumentParser) -> None:
           parser: The argument parser instance
         """
 
-        parser.add_argument('--prune', action='store_true', help='Delete any accounts with usernames not found in LDAP.')
+        parser.add_argument('--prune', action='store_true', help='Delete accounts with usernames not found in LDAP.')
 
     def handle(self, *args, **options) -> None:
         """Handle the command execution."""