Skip to content

Commit b053ed0

Browse files
fix: address PR review — sparkline order, periodic_task 500, N+1 queries, imports
- Sparkline: fetch newest 10 runs (order_by -end_time) then reverse for chart - Fix HTTP 500 for unscheduled jobs: use captured `periodic` variable instead of re-accessing task.periodic_task in response dict - Batch user queries: single User.objects.filter(id__in=...) instead of per-row User.objects.get() in get_triggered_by - Use Window(RowNumber) for run_number instead of loading all run IDs - Deduplicate parse_datetime import — moved to top-level
1 parent e719d6d commit b053ed0

2 files changed

Lines changed: 39 additions & 19 deletions

File tree

backend/backend/core/scheduler/serializer.py

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
11
from django.contrib.auth import get_user_model
2+
from django.db.models import Window, F
3+
from django.db.models.functions import RowNumber
24
from rest_framework import serializers
35

46
from backend.core.scheduler.models import TaskRunHistory
@@ -19,6 +21,21 @@ class Meta:
1921
model = TaskRunHistory
2022
fields = "__all__"
2123

24+
def _get_user_cache(self):
25+
"""Batch-load users for all runs in one query, cached per serializer instance."""
26+
if not hasattr(self, "_user_cache"):
27+
user_ids = set()
28+
for obj in self.instance if hasattr(self.instance, '__iter__') else [self.instance]:
29+
if obj and obj.kwargs and obj.kwargs.get("user_id"):
30+
user_ids.add(obj.kwargs["user_id"])
31+
if user_ids:
32+
self._user_cache = {
33+
str(u.id): u for u in User.objects.filter(id__in=user_ids)
34+
}
35+
else:
36+
self._user_cache = {}
37+
return self._user_cache
38+
2239
def get_duration(self, obj):
2340
"""Human-readable duration string."""
2441
if obj.start_time and obj.end_time:
@@ -46,32 +63,34 @@ def get_run_number(self, obj):
4663
self._run_number_cache = {}
4764
task_detail_id = obj.user_task_detail_id
4865
if task_detail_id not in self._run_number_cache:
49-
# Get all run IDs for this job ordered by start_time ASC
50-
run_ids = list(
66+
annotated = (
5167
TaskRunHistory.objects.filter(user_task_detail_id=task_detail_id)
52-
.order_by("start_time")
53-
.values_list("id", flat=True)
68+
.annotate(
69+
row_num=Window(
70+
expression=RowNumber(),
71+
order_by=F("start_time").asc(),
72+
)
73+
)
74+
.values_list("id", "row_num")
5475
)
55-
self._run_number_cache[task_detail_id] = {
56-
rid: idx + 1 for idx, rid in enumerate(run_ids)
57-
}
76+
self._run_number_cache[task_detail_id] = dict(annotated)
5877
return self._run_number_cache[task_detail_id].get(obj.id, 0)
5978

6079
def get_triggered_by(self, obj):
61-
"""Resolve user_id from kwargs to username."""
80+
"""Resolve user_id from kwargs to username using batch-loaded cache."""
6281
if not obj.kwargs:
6382
return None
6483
user_id = obj.kwargs.get("user_id")
6584
if not user_id:
6685
return None
67-
try:
68-
user = User.objects.get(id=user_id)
86+
cache = self._get_user_cache()
87+
user = cache.get(str(user_id))
88+
if user:
6989
return {
7090
"id": str(user.id),
7191
"username": user.get_full_name() or user.username or user.email,
7292
}
73-
except (User.DoesNotExist, ValueError):
74-
return {"id": str(user_id), "username": str(user_id)}
93+
return {"id": str(user_id), "username": str(user_id)}
7594

7695
def get_model_count(self, obj):
7796
"""Total model count from result."""

backend/backend/core/scheduler/views.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from datetime import timedelta
55

66
from django.utils import timezone
7+
from django.utils.dateparse import parse_datetime
78
from django_celery_beat.models import CrontabSchedule, IntervalSchedule, PeriodicTask
89
from rest_framework import status
910
from rest_framework.decorators import api_view, permission_classes
@@ -632,16 +633,18 @@ def run_stats(request, project_id, user_task_id):
632633
expected_duration_ms = int(sum(durations) / len(durations))
633634

634635
# Duration trend (last 10 completed runs for sparkline)
635-
recent_runs = runs.filter(
636+
recent_runs = list(runs.filter(
636637
start_time__isnull=False, end_time__isnull=False
637-
).order_by("end_time")[:10]
638+
).order_by("-end_time")[:10])
639+
recent_runs.reverse() # chronological order for sparkline
638640
duration_trend = [
639641
int((r.end_time - r.start_time).total_seconds() * 1000) for r in recent_runs
640642
]
641643

642644
# Schedule info
643645
schedule_type = None
644646
schedule_label = None
647+
periodic = None
645648
try:
646649
periodic = task.periodic_task
647650
if periodic:
@@ -653,7 +656,7 @@ def run_stats(request, project_id, user_task_id):
653656
schedule_type = "interval"
654657
schedule_label = f"Every {periodic.interval.every} {periodic.interval.period}"
655658
except Exception:
656-
pass
659+
periodic = None
657660

658661
return Response({
659662
"success": True,
@@ -676,7 +679,7 @@ def run_stats(request, project_id, user_task_id):
676679
},
677680
"schedule_type": schedule_type,
678681
"schedule_label": schedule_label,
679-
"schedule_enabled": task.periodic_task.enabled if task.periodic_task else False,
682+
"schedule_enabled": periodic.enabled if periodic else False,
680683
},
681684
}, status=status.HTTP_200_OK)
682685
except UserTaskDetails.DoesNotExist:
@@ -714,13 +717,11 @@ def task_run_history(request, project_id, user_task_id):
714717
if status_filter:
715718
runs = runs.filter(status=status_filter)
716719
if date_from:
717-
from django.utils.dateparse import parse_datetime
718720
dt = parse_datetime(date_from)
719721
if dt:
720722
runs = runs.filter(start_time__gte=dt)
721723
if date_to:
722-
from django.utils.dateparse import parse_datetime as parse_dt
723-
dt = parse_dt(date_to)
724+
dt = parse_datetime(date_to)
724725
if dt:
725726
runs = runs.filter(start_time__lte=dt)
726727
if search:

0 commit comments

Comments
 (0)