Skip to content

Commit b3d9199

Browse files
cshiels-ieclaude
andcommitted
Hardcode dashboard collection retention to 90 days with opt-in controller setting
The get_retention_days() function (which reads from the Controller's active cleanup_jobs schedules) was not fully tested. Gate it behind DASHBOARD_COLLECTION['USE_CONTROLLER_RETENTION'] (default False) so both the initial backfill window and the daily cleanup task always use the fixed 90-day default until the setting is explicitly enabled. Enable via settings.yaml or METRICS_SERVICE_DASHBOARD_COLLECTION__USE_CONTROLLER_RETENTION=true. Assisted-by: Claude Code (claude-sonnet-4-6) Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent 65a8cfe commit b3d9199

3 files changed

Lines changed: 61 additions & 18 deletions

File tree

apps/dashboard_reports/tasks.py

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,12 @@ def _get_renamed_kwarg(kwargs: dict, new_key: str, old_key: str, task_name: str)
5555
return _MISSING
5656

5757

58+
def _use_controller_retention() -> bool:
59+
"""Return True when DASHBOARD_COLLECTION['USE_CONTROLLER_RETENTION'] is set in settings."""
60+
dashboard_cfg = getattr(settings, "DASHBOARD_COLLECTION", None) or {}
61+
return bool(dashboard_cfg.get("USE_CONTROLLER_RETENTION", False))
62+
63+
5864
def get_retention_days(db_name: str = DEFAULT_AWX_DB_NAME) -> int:
5965
"""
6066
Return the effective retention period in days derived from active AWX cleanup schedules.
@@ -196,9 +202,10 @@ def _sync_jobs_atomically(job_results: list) -> list:
196202
def _resolve_collection_params(task_name: str, kwargs: dict) -> tuple[str, datetime, datetime, int]:
197203
"""Resolve db_name, since, until, and batch_size from task kwargs with defaults applied.
198204
199-
When ``since`` is not provided and no prior ``JobData`` timestamp exists, the initial
200-
retention window is determined by ``get_retention_days`` (queried from active AWX cleanup
201-
schedules) rather than a static setting.
205+
When ``since`` is not provided and no prior ``JobData`` timestamp exists, the backfill
206+
window defaults to ``DEFAULT_RETENTION_DAYS`` (90 days). When the
207+
``DASHBOARD_COLLECTION['USE_CONTROLLER_RETENTION']`` setting is enabled, the window is
208+
derived from active AWX cleanup_jobs schedules via ``get_retention_days`` instead.
202209
"""
203210
dashboard_cfg = getattr(settings, "DASHBOARD_COLLECTION", None) or {}
204211
db_name = _get_renamed_kwarg(kwargs, "awx_database", "database", task_name)
@@ -209,9 +216,12 @@ def _resolve_collection_params(task_name: str, kwargs: dict) -> tuple[str, datet
209216
# used correctly and no jobs are missed in the gap between MAX(finished) and MAX(awx_modified).
210217
since = _parse_dt(kwargs.get("since")) or JobData.last_finished_timestamp()
211218
if since is None:
212-
retention_days = get_retention_days(db_name)
213-
if retention_days <= 0:
214-
raise ValueError(f"Retention days must be > 0, got {retention_days!r}")
219+
if _use_controller_retention():
220+
retention_days = get_retention_days(db_name)
221+
if retention_days <= 0:
222+
raise ValueError(f"Retention days must be > 0, got {retention_days!r}")
223+
else:
224+
retention_days = DEFAULT_RETENTION_DAYS
215225
since = (
216226
(until - timedelta(days=retention_days)).replace(hour=0, minute=0, second=0, microsecond=0).astimezone(UTC)
217227
)
@@ -614,7 +624,8 @@ def cleanup_dashboard_reports_old_data(**kwargs) -> dict[str, Any]:
614624
db_name = _get_renamed_kwarg(kwargs, "awx_database", "database", task_name)
615625
db_name = DEFAULT_AWX_DB_NAME if db_name is _MISSING else db_name
616626
retention_days = _get_renamed_kwarg(kwargs, "retention_days", "retention_period_days", task_name)
617-
retention_days = get_retention_days(db_name) if retention_days is _MISSING else retention_days
627+
if retention_days is _MISSING:
628+
retention_days = get_retention_days(db_name) if _use_controller_retention() else DEFAULT_RETENTION_DAYS
618629
retention_days, error_result = _normalize_retention_days(task_name, retention_days)
619630
if error_result is not None:
620631
return error_result

apps/settings/defaults.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,9 @@
180180
# Dashboard collection schedule configuration
181181
DASHBOARD_COLLECTION = {
182182
"COLLECTION_SCHEDULE_CRON": "0 */6 * * *",
183+
# Set to True to derive the backfill/retention window from the Controller's active
184+
# cleanup_jobs schedules instead of the fixed 90-day default.
185+
"USE_CONTROLLER_RETENTION": False,
183186
}
184187

185188
# Conditional static files directory (avoids staticfiles.W004 when absent)

tests/unit/dashboard_reports/test_tasks.py

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -508,6 +508,12 @@ def mock_retention_days(self):
508508
with patch("apps.dashboard_reports.tasks.get_retention_days", return_value=DEFAULT_RETENTION_DAYS) as mock:
509509
yield mock
510510

511+
@pytest.fixture
512+
def with_controller_retention(self):
513+
"""Enable the controller-retention path so get_retention_days() is called."""
514+
with patch("apps.dashboard_reports.tasks._use_controller_retention", return_value=True):
515+
yield
516+
511517
def test_cleanup_success(self, mock_jobdata_objects, mock_log_task_execution, mock_create_task_result_cleanup):
512518
"""Successful deletion returns a success result with the correct record count."""
513519
mock_jobdata_objects.objects.filter.return_value.count.return_value = 5
@@ -538,33 +544,53 @@ def test_cleanup_exception(self, mock_jobdata_objects, mock_log_task_execution,
538544
assert "Cleanup failed" in kwargs["error"]
539545

540546
def test_cleanup_defaults_to_get_retention_days(
541-
self, mock_retention_days, mock_jobdata_objects, mock_log_task_execution, mock_create_task_result_cleanup
547+
self,
548+
with_controller_retention,
549+
mock_retention_days,
550+
mock_jobdata_objects,
551+
mock_log_task_execution,
552+
mock_create_task_result_cleanup,
542553
):
543-
"""When retention_days is omitted, get_retention_days() determines the default."""
554+
"""When controller retention is enabled and retention_days is omitted, get_retention_days() determines the default."""
544555
mock_retention_days.return_value = 60
545556
mock_jobdata_objects.objects.filter.return_value.count.return_value = 0
546557
cleanup_dashboard_reports_old_data()
547558
_, kwargs = mock_create_task_result_cleanup.call_args
548559
assert kwargs["data"]["retention_days"] == 60
549560

550561
def test_cleanup_passes_db_name_to_get_retention_days(
551-
self, mock_retention_days, mock_jobdata_objects, mock_log_task_execution, mock_create_task_result_cleanup
562+
self,
563+
with_controller_retention,
564+
mock_retention_days,
565+
mock_jobdata_objects,
566+
mock_log_task_execution,
567+
mock_create_task_result_cleanup,
552568
):
553569
"""The 'awx_database' kwarg is forwarded to get_retention_days so retention and collection use the same connection."""
554570
mock_jobdata_objects.objects.filter.return_value.count.return_value = 0
555571
cleanup_dashboard_reports_old_data(awx_database="custom_db")
556572
mock_retention_days.assert_called_once_with("custom_db")
557573

558574
def test_cleanup_passes_default_db_name_when_no_database_kwarg(
559-
self, mock_retention_days, mock_jobdata_objects, mock_log_task_execution, mock_create_task_result_cleanup
575+
self,
576+
with_controller_retention,
577+
mock_retention_days,
578+
mock_jobdata_objects,
579+
mock_log_task_execution,
580+
mock_create_task_result_cleanup,
560581
):
561582
"""When 'awx_database' kwarg is absent, get_retention_days is called with the 'awx' alias."""
562583
mock_jobdata_objects.objects.filter.return_value.count.return_value = 0
563584
cleanup_dashboard_reports_old_data()
564585
mock_retention_days.assert_called_once_with("awx")
565586

566587
def test_cleanup_deprecated_database_kwarg_still_honored(
567-
self, mock_retention_days, mock_jobdata_objects, mock_log_task_execution, mock_create_task_result_cleanup
588+
self,
589+
with_controller_retention,
590+
mock_retention_days,
591+
mock_jobdata_objects,
592+
mock_log_task_execution,
593+
mock_create_task_result_cleanup,
568594
):
569595
"""The pre-rename 'database' kwarg still resolves db_name rather than being silently dropped."""
570596
mock_jobdata_objects.objects.filter.return_value.count.return_value = 0
@@ -690,10 +716,11 @@ def test_since_falls_back_to_last_timestamp(self, mock_jobdata):
690716
_, got_since, _, _ = _resolve_collection_params("collect_dashboard_reports_data", {})
691717
assert got_since == ts
692718

719+
@patch("apps.dashboard_reports.tasks._use_controller_retention", return_value=True)
693720
@patch("apps.dashboard_reports.tasks.get_retention_days", return_value=30)
694721
@patch("apps.dashboard_reports.tasks.JobData")
695-
def test_since_computed_from_retention_days_when_no_timestamp(self, mock_jobdata, mock_retention):
696-
"""When since is absent and last_timestamp() is None, compute since from get_retention_days()."""
722+
def test_since_computed_from_retention_days_when_no_timestamp(self, mock_jobdata, mock_retention, _mock_use_ctrl):
723+
"""When controller retention is enabled and last_timestamp() is None, compute since from get_retention_days()."""
697724
from django.test import override_settings
698725

699726
mock_jobdata.last_finished_timestamp.return_value = None
@@ -709,10 +736,11 @@ def test_since_computed_from_retention_days_when_no_timestamp(self, mock_jobdata
709736
assert batch_size == 500
710737
mock_retention.assert_called_once_with("awx")
711738

739+
@patch("apps.dashboard_reports.tasks._use_controller_retention", return_value=True)
712740
@patch("apps.dashboard_reports.tasks.get_retention_days", return_value=30)
713741
@patch("apps.dashboard_reports.tasks.JobData")
714-
def test_custom_database_kwarg_forwarded_to_retention(self, mock_jobdata, mock_retention):
715-
"""When 'awx_database' kwarg is given and no prior timestamp exists, db_name is passed to get_retention_days."""
742+
def test_custom_database_kwarg_forwarded_to_retention(self, mock_jobdata, mock_retention, _mock_use_ctrl):
743+
"""When controller retention is enabled and no prior timestamp exists, db_name is passed to get_retention_days."""
716744
mock_jobdata.last_finished_timestamp.return_value = None
717745
until = datetime(2024, 6, 1, tzinfo=UTC)
718746
_resolve_collection_params(
@@ -746,10 +774,11 @@ def test_invalid_batch_size_raises_value_error(self, mock_jobdata):
746774
):
747775
_resolve_collection_params("collect_dashboard_reports_data", {})
748776

777+
@patch("apps.dashboard_reports.tasks._use_controller_retention", return_value=True)
749778
@patch("apps.dashboard_reports.tasks.get_retention_days", return_value=0)
750779
@patch("apps.dashboard_reports.tasks.JobData")
751-
def test_zero_retention_days_raises_value_error(self, mock_jobdata, mock_retention):
752-
"""get_retention_days returning 0 raises ValueError (retention must be > 0)."""
780+
def test_zero_retention_days_raises_value_error(self, mock_jobdata, mock_retention, _mock_use_ctrl):
781+
"""When controller retention is enabled, get_retention_days returning 0 raises ValueError."""
753782
mock_jobdata.last_finished_timestamp.return_value = None
754783
with pytest.raises(ValueError, match="Retention days must be > 0"):
755784
_resolve_collection_params("collect_dashboard_reports_data", {})

0 commit comments

Comments
 (0)