Fix elapsed_time_expression() for jobs completed without an execution time
The expression coalesced to Now() - started with no regard for whether the job had finished, so a row with both started and completed set but a null execution_time resolved to an ever-growing interval, while the elapsed_time property returned None for the same row. Sorting the jobs table descending by execution time therefore ranked those rows above every real value. Gate the live branch on completed__isnull=True so the expression agrees with the property. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
parent
5346dab1c3
commit
f427e61063
|
|
@ -11,7 +11,7 @@ from django.core.exceptions import ValidationError
|
|||
from django.core.serializers.json import DjangoJSONEncoder
|
||||
from django.core.validators import MinValueValidator
|
||||
from django.db import models, transaction
|
||||
from django.db.models import ExpressionWrapper, F
|
||||
from django.db.models import Case, ExpressionWrapper, F, When
|
||||
from django.db.models.functions import Coalesce, Now
|
||||
from django.urls import reverse
|
||||
from django.utils import timezone
|
||||
|
|
@ -200,11 +200,19 @@ class Job(models.Model):
|
|||
def elapsed_time_expression():
|
||||
"""
|
||||
A queryset expression mirroring the `elapsed_time` property, for use in ordering and
|
||||
filtering. Resolves to null for jobs which have not yet started.
|
||||
filtering. Resolves to null for jobs which have not yet started, and for jobs which have
|
||||
completed without recording an execution time.
|
||||
"""
|
||||
return Coalesce(
|
||||
'execution_time',
|
||||
ExpressionWrapper(Now() - F('started'), output_field=models.DurationField()),
|
||||
# Only a job which has yet to complete accrues elapsed time
|
||||
Case(
|
||||
When(
|
||||
completed__isnull=True,
|
||||
then=ExpressionWrapper(Now() - F('started'), output_field=models.DurationField()),
|
||||
),
|
||||
output_field=models.DurationField(),
|
||||
),
|
||||
)
|
||||
|
||||
def delete(self, *args, **kwargs):
|
||||
|
|
|
|||
|
|
@ -418,6 +418,20 @@ class JobTestCase(TestCase):
|
|||
self.assertIsNone(job.started)
|
||||
self.assertIsNone(job.elapsed_time)
|
||||
|
||||
def test_elapsed_time_none_when_completed_without_execution_time(self):
|
||||
"""
|
||||
A job which completed without recording an execution time (e.g. one predating the field)
|
||||
has no elapsed time to report; it must not accrue time indefinitely.
|
||||
"""
|
||||
job = self._make_job(None, JobNotificationChoices.NOTIFICATION_NEVER)
|
||||
job.started = timezone.now() - timedelta(seconds=90)
|
||||
job.completed = timezone.now()
|
||||
job.status = JobStatusChoices.STATUS_COMPLETED
|
||||
job.save()
|
||||
|
||||
self.assertIsNone(job.execution_time)
|
||||
self.assertIsNone(job.elapsed_time)
|
||||
|
||||
def test_elapsed_time_expression_matches_property(self):
|
||||
"""
|
||||
The elapsed_time_expression() queryset expression should agree with the elapsed_time
|
||||
|
|
@ -430,6 +444,14 @@ class JobTestCase(TestCase):
|
|||
completed.status = JobStatusChoices.STATUS_COMPLETED
|
||||
completed.save()
|
||||
|
||||
# A job which completed without recording an execution time must resolve to null, rather
|
||||
# than to an ever-growing interval since it started
|
||||
unrecorded = self._make_job(None, JobNotificationChoices.NOTIFICATION_NEVER)
|
||||
unrecorded.started = timezone.now() - timedelta(seconds=90)
|
||||
unrecorded.completed = timezone.now()
|
||||
unrecorded.status = JobStatusChoices.STATUS_COMPLETED
|
||||
unrecorded.save()
|
||||
|
||||
running = self._make_job(None, JobNotificationChoices.NOTIFICATION_NEVER)
|
||||
running.started = timezone.now() - timedelta(minutes=5)
|
||||
running.save()
|
||||
|
|
@ -444,6 +466,7 @@ class JobTestCase(TestCase):
|
|||
}
|
||||
|
||||
self.assertEqual(annotated[completed.pk].elapsed, timedelta(seconds=90))
|
||||
self.assertIsNone(annotated[unrecorded.pk].elapsed)
|
||||
self.assertIsNone(annotated[pending.pk].elapsed)
|
||||
# The running job's elapsed time is computed at query time, so compare approximately
|
||||
self.assertAlmostEqual(
|
||||
|
|
|
|||
Loading…
Reference in New Issue