-
Notifications
You must be signed in to change notification settings - Fork 7
feat: redesign Run History page with stats dashboard, row counts, and job switcher #74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 7 commits
a5197b4
599c6a4
8dff116
35e1353
9aaad8e
7e5ecc4
e719d6d
b053ed0
9cde74f
c8799f5
0f2df1a
6240104
a525d56
147e654
25470e6
923d615
6d9ee7c
7ae287c
85ee5d9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,96 @@ | ||
| from django.contrib.auth import get_user_model | ||
| from rest_framework import serializers | ||
|
|
||
| from backend.core.scheduler.models import TaskRunHistory | ||
|
|
||
| User = get_user_model() | ||
|
|
||
|
|
||
| class TaskRunHistorySerializer(serializers.ModelSerializer): | ||
| duration = serializers.SerializerMethodField() | ||
| duration_ms = serializers.SerializerMethodField() | ||
| run_number = serializers.SerializerMethodField() | ||
| triggered_by = serializers.SerializerMethodField() | ||
| model_count = serializers.SerializerMethodField() | ||
| failed_models = serializers.SerializerMethodField() | ||
| skipped_count = serializers.SerializerMethodField() | ||
|
|
||
| class Meta: | ||
| model = TaskRunHistory | ||
| fields = "__all__" # Include all fields or specify fields like ['id', 'start_time', 'end_time', 'status'] | ||
| fields = "__all__" | ||
|
wicky-zipstack marked this conversation as resolved.
Outdated
|
||
|
|
||
| def get_duration(self, obj): | ||
| """Calculate duration (end_time - start_time)""" | ||
| """Human-readable duration string.""" | ||
| if obj.start_time and obj.end_time: | ||
| delta = obj.end_time - obj.start_time | ||
| total_ms = int(delta.total_seconds() * 1000) | ||
| if total_ms < 1000: | ||
| return f"{total_ms}ms" | ||
| elif total_ms < 60000: | ||
| return f"{total_ms / 1000:.1f}s" | ||
| else: | ||
| minutes = total_ms // 60000 | ||
| seconds = (total_ms % 60000) / 1000 | ||
| return f"{minutes}m {seconds:.0f}s" | ||
| return None | ||
|
|
||
| def get_duration_ms(self, obj): | ||
| """Duration in milliseconds for sorting/comparison.""" | ||
| if obj.start_time and obj.end_time: | ||
| return str(obj.end_time - obj.start_time) # Convert timedelta to string | ||
| return None # If end_time is missing, return None | ||
| return int((obj.end_time - obj.start_time).total_seconds() * 1000) | ||
| return None | ||
|
|
||
| def get_run_number(self, obj): | ||
| """Sequential run number within the job (1 = oldest).""" | ||
| if not hasattr(self, "_run_number_cache"): | ||
| self._run_number_cache = {} | ||
| task_detail_id = obj.user_task_detail_id | ||
| if task_detail_id not in self._run_number_cache: | ||
| # Get all run IDs for this job ordered by start_time ASC | ||
| run_ids = list( | ||
| TaskRunHistory.objects.filter(user_task_detail_id=task_detail_id) | ||
| .order_by("start_time") | ||
| .values_list("id", flat=True) | ||
| ) | ||
| self._run_number_cache[task_detail_id] = { | ||
| rid: idx + 1 for idx, rid in enumerate(run_ids) | ||
|
greptile-apps[bot] marked this conversation as resolved.
Outdated
|
||
| } | ||
| return self._run_number_cache[task_detail_id].get(obj.id, 0) | ||
|
|
||
| def get_triggered_by(self, obj): | ||
| """Resolve user_id from kwargs to username.""" | ||
| if not obj.kwargs: | ||
| return None | ||
| user_id = obj.kwargs.get("user_id") | ||
| if not user_id: | ||
| return None | ||
| try: | ||
| user = User.objects.get(id=user_id) | ||
| return { | ||
| "id": str(user.id), | ||
| "username": user.get_full_name() or user.username or user.email, | ||
| } | ||
| except (User.DoesNotExist, ValueError): | ||
| return {"id": str(user_id), "username": str(user_id)} | ||
|
greptile-apps[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
|
wicky-zipstack marked this conversation as resolved.
|
||
| def get_model_count(self, obj): | ||
| """Total model count from result.""" | ||
| if obj.result and isinstance(obj.result, dict): | ||
| return obj.result.get("total", 0) | ||
| return 0 | ||
|
|
||
| def get_failed_models(self, obj): | ||
| """List of failed model names.""" | ||
| if obj.result and isinstance(obj.result, dict): | ||
| models = obj.result.get("models", []) | ||
| return [m["name"] for m in models if m.get("end_status") == "FAIL" or m.get("status") == "failure"] | ||
| return [] | ||
|
|
||
| def get_skipped_count(self, obj): | ||
| """Count of skipped models (total - passed - failed).""" | ||
| if obj.result and isinstance(obj.result, dict): | ||
| total = obj.result.get("total", 0) | ||
| passed = obj.result.get("passed", 0) | ||
| failed = obj.result.get("failed", 0) | ||
| return max(0, total - passed - failed) | ||
| return 0 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -583,6 +583,109 @@ | |
| ) | ||
|
|
||
|
|
||
| @api_view(["GET"]) | ||
| @permission_classes([IsAuthenticated]) | ||
| def run_stats(request, project_id, user_task_id): | ||
| """Get aggregated run statistics for a job — stats cards data.""" | ||
| try: | ||
| query = {"id": user_task_id} | ||
| if _is_valid_project_id(project_id): | ||
| query["project__project_uuid"] = project_id | ||
|
wicky-zipstack marked this conversation as resolved.
|
||
| task = UserTaskDetails.objects.get(**query) | ||
| runs = TaskRunHistory.objects.filter(user_task_detail=task) | ||
|
|
||
| now = timezone.now() | ||
| last_7d = now - timedelta(days=7) | ||
| last_24h = now - timedelta(hours=24) | ||
| prev_24h_start = now - timedelta(hours=48) | ||
|
|
||
| # Success rate (7 days) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3 —
Two reasonable normalizations:
At minimum, the metric's definition should be documented next to the API or in the UI tooltip so users know what they're looking at.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 147e654 — denominator now only counts completed runs (
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 147e654 — denominator now only counts completed runs (
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 147e654 — denominator now only counts completed runs (SUCCESS/FAILURE). In-progress runs excluded. |
||
| runs_7d = runs.filter(start_time__gte=last_7d) | ||
| total_7d = runs_7d.count() | ||
| success_7d = runs_7d.filter(status="SUCCESS").count() | ||
| success_rate = round((success_7d / total_7d * 100), 1) if total_7d > 0 else None | ||
|
|
||
| # Average duration (successful runs, 7 days) | ||
| successful_runs_7d = runs_7d.filter(status="SUCCESS", start_time__isnull=False, end_time__isnull=False) | ||
| avg_duration_ms = None | ||
| if successful_runs_7d.exists(): | ||
| durations = [(r.end_time - r.start_time).total_seconds() * 1000 for r in successful_runs_7d] | ||
| avg_duration_ms = int(sum(durations) / len(durations)) | ||
|
|
||
| # Failures (24h) + comparison with previous 24h | ||
| failures_24h = runs.filter(start_time__gte=last_24h, status="FAILURE").count() | ||
| failures_prev_24h = runs.filter( | ||
| start_time__gte=prev_24h_start, start_time__lt=last_24h, status="FAILURE" | ||
| ).count() | ||
|
|
||
| # Last successful run | ||
| last_success = runs.filter(status="SUCCESS").order_by("-end_time").first() | ||
| last_success_time = last_success.end_time if last_success else None | ||
|
|
||
| # Expected duration (avg of last 5 successful runs) | ||
| recent_successes = runs.filter( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2 — Aggregations done in Python, not SQL Both from django.db.models import Avg, F, ExpressionWrapper, DurationField, Count, Case, When
from django.db.models.functions import Extract
avg_dur = successful_runs_7d.aggregate(
avg=Avg(ExpressionWrapper(F("end_time") - F("start_time"), output_field=DurationField()))
)["avg"]
avg_duration_ms = int(avg_dur.total_seconds() * 1000) if avg_dur else NoneMore broadly, this endpoint fires 8+ separate queries (
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid point. For the current scale (most jobs have <1k weekly runs), the Python-side aggregation works fine. Will refactor to Django ORM
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid point. For current scale (most jobs <1k weekly runs), Python-side aggregation works fine. Will refactor to Django ORM Aggregate calls in a follow-up optimization pass. Noted.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Valid point. Current scale works fine. Will refactor to Django ORM Aggregate calls in a follow-up optimization pass. |
||
| status="SUCCESS", start_time__isnull=False, end_time__isnull=False | ||
| ).order_by("-end_time")[:5] | ||
| expected_duration_ms = None | ||
| if recent_successes.exists(): | ||
| durations = [(r.end_time - r.start_time).total_seconds() * 1000 for r in recent_successes] | ||
| expected_duration_ms = int(sum(durations) / len(durations)) | ||
|
|
||
| # Duration trend (last 10 completed runs for sparkline) | ||
| recent_runs = runs.filter( | ||
| start_time__isnull=False, end_time__isnull=False | ||
| ).order_by("end_time")[:10] | ||
| duration_trend = [ | ||
| int((r.end_time - r.start_time).total_seconds() * 1000) for r in recent_runs | ||
| ] | ||
|
|
||
| # Schedule info | ||
| schedule_type = None | ||
| schedule_label = None | ||
| try: | ||
| periodic = task.periodic_task | ||
| if periodic: | ||
| if periodic.crontab: | ||
| schedule_type = "cron" | ||
| c = periodic.crontab | ||
| schedule_label = f"{c.minute} {c.hour} {c.day_of_week}" | ||
|
greptile-apps[bot] marked this conversation as resolved.
Outdated
|
||
| elif periodic.interval: | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
| schedule_type = "interval" | ||
| schedule_label = f"Every {periodic.interval.every} {periodic.interval.period}" | ||
| except Exception: | ||
| pass | ||
|
|
||
| return Response({ | ||
| "success": True, | ||
| "data": { | ||
| "success_rate_7d": success_rate, | ||
| "success_count_7d": success_7d, | ||
| "total_count_7d": total_7d, | ||
| "avg_duration_ms": avg_duration_ms, | ||
| "failures_24h": failures_24h, | ||
| "failures_prev_24h": failures_prev_24h, | ||
| "failures_change": failures_24h - failures_prev_24h, | ||
| "last_successful_run": last_success_time, | ||
| "expected_duration_ms": expected_duration_ms, | ||
| "duration_trend": duration_trend, | ||
| "total_runs": runs.count(), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2 — "total_runs": runs.count(),For a job that's been running every 5 minutes for a year, this is a 100k+ row COUNT on every page load. A grep of the frontend confirms
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This field is used by the FE for run_number computation (
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This field is needed — the FE uses it for run_number computation ( |
||
| "job_name": task.task_name, | ||
| "environment": { | ||
| "name": task.environment.environment_name if task.environment else None, | ||
| "type": task.environment.deployment_type if task.environment else None, | ||
| }, | ||
| "schedule_type": schedule_type, | ||
| "schedule_label": schedule_label, | ||
| "schedule_enabled": task.periodic_task.enabled if task.periodic_task else False, | ||
| }, | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
| }, status=status.HTTP_200_OK) | ||
| except UserTaskDetails.DoesNotExist: | ||
| return Response({"error": "Task not found"}, status=status.HTTP_404_NOT_FOUND) | ||
| except Exception as e: | ||
| logger.error(f"Error getting run stats: {e}") | ||
| return Response({"error": str(e)}, status=status.HTTP_500_INTERNAL_SERVER_ERROR) | ||
Check warningCode scanning / CodeQL Information exposure through an exception Medium Stack trace information Error loading related location Loading |
||
|
github-advanced-security[bot] marked this conversation as resolved.
Fixed
|
||
|
|
||
|
|
||
| @api_view(["GET"]) | ||
| @permission_classes([IsAuthenticated]) | ||
| def task_run_history(request, project_id, user_task_id): | ||
|
|
@@ -600,12 +703,28 @@ | |
| trigger_filter = request.GET.get("trigger") | ||
| scope_filter = request.GET.get("scope") | ||
| status_filter = request.GET.get("status") | ||
| date_from = request.GET.get("date_from") | ||
| date_to = request.GET.get("date_to") | ||
| search = request.GET.get("search") | ||
|
|
||
| if trigger_filter: | ||
| runs = runs.filter(trigger=trigger_filter) | ||
| if scope_filter: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3 — scope_filter = request.GET.get("scope")
...
if scope_filter:
runs = runs.filter(scope=scope_filter)The redesigned filter bar dropped the Scope select, so the frontend no longer sends
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kept intentionally for backward compatibility — the BE still supports the scope filter if any other client sends it. Harmless dead path.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Kept for backward compatibility — harmless dead path, BE still supports it if other clients send it. |
||
| runs = runs.filter(scope=scope_filter) | ||
| if status_filter: | ||
| runs = runs.filter(status=status_filter) | ||
| if date_from: | ||
| from django.utils.dateparse import parse_datetime | ||
| dt = parse_datetime(date_from) | ||
| if dt: | ||
| runs = runs.filter(start_time__gte=dt) | ||
| if date_to: | ||
| from django.utils.dateparse import parse_datetime as parse_dt | ||
| dt = parse_dt(date_to) | ||
| if dt: | ||
| runs = runs.filter(start_time__lte=dt) | ||
|
greptile-apps[bot] marked this conversation as resolved.
|
||
| if search: | ||
| runs = runs.filter(error_message__icontains=search) | ||
|
|
||
| runs = runs.order_by("-start_time") | ||
| total = runs.count() | ||
|
|
@@ -620,6 +739,7 @@ | |
| "page_items": { | ||
| "id": task.id, | ||
| "job_name": task.task_name, | ||
| "project_id": str(task.project.project_uuid) if task.project else None, | ||
| "env_type": task.environment.deployment_type | ||
| if task.environment | ||
| else None, | ||
|
|
@@ -705,9 +825,10 @@ | |
| synchronous (in-process) execution so local dev works without Redis. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Must Fix: When query = {"id": user_task_id}
if _is_valid_project_id(project_id):
query["project__project_uuid"] = project_id
task = UserTaskDetails.objects.get(**query)This means any authenticated user can trigger any job by ID regardless of project ownership. If user A doesn't have access to project B's jobs, they can still trigger them by knowing the task ID and passing Fix: When if _is_valid_project_id(project_id):
query["project__project_uuid"] = project_id
else:
# _all: restrict to projects the user has access to
user_projects = ProjectDetails.objects.filter(
organization_id=request.user.organization_id
).values_list('project_uuid', flat=True)
query["project__project_uuid__in"] = user_projectsOr at minimum, verify the user has access to the task's project after fetching it.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already mitigated — |
||
| """ | ||
| try: | ||
| task = UserTaskDetails.objects.get( | ||
| id=user_task_id, project__project_uuid=project_id | ||
| ) | ||
| query = {"id": user_task_id} | ||
| if _is_valid_project_id(project_id): | ||
| query["project__project_uuid"] = project_id | ||
| task = UserTaskDetails.objects.get(**query) | ||
| except UserTaskDetails.DoesNotExist: | ||
| return Response( | ||
| {"error": "Task not found"}, status=status.HTTP_404_NOT_FOUND | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.