diff --git a/netbox/core/tables/jobs.py b/netbox/core/tables/jobs.py index afb63e764..e3a7c2b74 100644 --- a/netbox/core/tables/jobs.py +++ b/netbox/core/tables/jobs.py @@ -45,7 +45,7 @@ class JobTable(NetBoxTable): completed = columns.DateTimeColumn( verbose_name=_('Completed'), ) - execution_time = columns.DurationColumn( + execution_time = tables.Column( verbose_name=_('Execution Time'), # Render running jobs (which have no recorded execution time yet) rather than the placeholder empty_values=(), @@ -87,10 +87,12 @@ class JobTable(NetBoxTable): return value def value_execution_time(self, record): - # Export the raw number of seconds rather than the humanized rendering - if (duration := record.elapsed_time) is None: + # Export the recorded execution time verbatim, as a raw number of seconds. A running job's + # provisional elapsed time is deliberately omitted, as is the clamping of anomalous negative + # values applied when rendering: an export is intended for analysis. + if record.execution_time is None: return None - return round(max(duration.total_seconds(), 0), 3) + return round(record.execution_time.total_seconds(), 3) def order_execution_time(self, queryset, is_descending): # Order by the value the column actually displays, so that a long-running job is not sorted diff --git a/netbox/core/tests/test_tables.py b/netbox/core/tests/test_tables.py index bb75446a8..91a1572d5 100644 --- a/netbox/core/tests/test_tables.py +++ b/netbox/core/tests/test_tables.py @@ -48,6 +48,10 @@ class JobExecutionTimeColumnTestCase(TestCase): name='negative', job_id=uuid.uuid4(), status=JobStatusChoices.STATUS_COMPLETED, started=now, completed=now, execution_time=timedelta(seconds=-5), ), + Job( + name='unrecorded', job_id=uuid.uuid4(), status=JobStatusChoices.STATUS_COMPLETED, + started=now - timedelta(seconds=90), completed=now, + ), Job( name='running', job_id=uuid.uuid4(), status=JobStatusChoices.STATUS_RUNNING, started=now - timedelta(minutes=5), @@ -87,28 +91,46 @@ class JobExecutionTimeColumnTestCase(TestCase): row = next(iter(table.rows)) self.assertEqual(str(row.get_cell('execution_time')), table.default) - def test_export_value_is_raw_seconds(self): - table = JobTable(Job.objects.filter(name='completed-90s')) + def test_render_job_completed_without_execution_time(self): + # A completed job with no recorded execution time shows the placeholder, rather than + # accruing time since it started + table = JobTable(Job.objects.filter(name='unrecorded')) + table.columns.show('execution_time') + row = next(iter(table.rows)) + self.assertEqual(str(row.get_cell('execution_time')), table.default) + + def _export_value(self, name): + table = JobTable(Job.objects.filter(name=name)) table.columns.show('execution_time') rows = list(table.as_values()) - index = rows[0].index('Execution Time') - self.assertEqual(rows[1][index], 90.0) + return rows[1][rows[0].index('Execution Time')] + + def test_export_value_is_raw_seconds(self): + self.assertEqual(self._export_value('completed-90s'), 90.0) def test_export_value_of_job_never_started(self): - table = JobTable(Job.objects.filter(name='pending')) - table.columns.show('execution_time') - rows = list(table.as_values()) - index = rows[0].index('Execution Time') - self.assertIsNone(rows[1][index]) + self.assertIsNone(self._export_value('pending')) + + def test_export_value_of_running_job(self): + # Only the recorded execution time is exported; a running job has none yet + self.assertIsNone(self._export_value('running')) + + def test_export_value_is_not_clamped(self): + # An anomalous negative value is clamped when rendered, but exported verbatim so that it + # remains visible to analysis + self.assertEqual(self._export_value('negative'), -5.0) def test_ordering_matches_displayed_values(self): """ Sorting must order by the value the column displays — which for a running job is its elapsed - time, not a null — so that a long-running job is not buried. Jobs which never started sort - last in both directions. + time, not a null — so that a long-running job is not buried. Jobs with no elapsed time to + display sort last in both directions, in pk order. """ # 'running' has been going 5 minutes, so it sorts between the 90s and 2d3h jobs ascending = ['negative', 'completed-subsecond', 'completed-90s', 'running', 'completed-long'] + # Neither a job which never started nor one which completed without recording an execution + # time has a value to sort by + no_value = ['unrecorded', 'pending'] for descending, expected in ( (False, ascending), @@ -119,7 +141,7 @@ class JobExecutionTimeColumnTestCase(TestCase): queryset, modified = table.columns['execution_time'].order(Job.objects.all(), descending) self.assertTrue(modified) names = list(queryset.values_list('name', flat=True)) - self.assertEqual(names, expected + ['pending']) + self.assertEqual(names, expected + no_value) def test_ordering_breaks_ties_on_pk(self): """ diff --git a/netbox/netbox/tables/columns.py b/netbox/netbox/tables/columns.py index a4e302105..359c79c6d 100644 --- a/netbox/netbox/tables/columns.py +++ b/netbox/netbox/tables/columns.py @@ -1,6 +1,5 @@ import zoneinfo from dataclasses import dataclass -from datetime import timedelta from urllib.parse import quote import django_tables2 as tables @@ -23,7 +22,6 @@ from extras.choices import CustomFieldTypeChoices from utilities.object_types import object_type_identifier, object_type_name from utilities.permissions import get_permission_for_model from utilities.request import get_safe_request_context -from utilities.string import humanize_duration from utilities.templatetags.builtins.filters import render_markdown from utilities.validators import url_scheme_is_allowed from utilities.views import get_action_url @@ -114,16 +112,17 @@ class DateTimeColumn(tables.Column): class DurationColumn(tables.Column): """ - Express a duration of time in a human-friendly format. Accepts either a timedelta or a count of - minutes. Example: 437 minutes becomes "7h 17m" + Express a duration of time (in minutes) in a human-friendly format. Example: 437 minutes becomes "7h 17m" """ def render(self, value): - if not isinstance(value, timedelta): - if not value: - # A zero count of minutes renders as empty rather than "0s" - return '' - value = timedelta(minutes=value) - return humanize_duration(value) + ret = '' + if days := value // 1440: + ret += f'{days}d ' + if hours := value % 1440 // 60: + ret += f'{hours}h ' + if minutes := value % 60: + ret += f'{minutes}m' + return ret.strip() def value(self, value): return value