fix(backend): fix review findings on the task manager retention settings [INFP-507]

Merged
SA
7d ago
+1390-309

Fixes for review findings on opsmill/infrahub#10862 (PR 1 of the INFP-507 stack). This PR targets task-history-retention-infp-507, so the author can merge it into #10862.

Why

A code review of #10862 found cases where the retention settings do not apply, or where task runs or events are deleted earlier than the operator expects. This PR fixes the must-fix and should-fix findings. Each finding was checked against the code and the Prefect 3.8.6 source before it was fixed, and each fix was checked again afterwards.

Non-goals:

  • No change to PRs 2 to 6. They need a rebase after this lands (see "Stack impact").
  • The infrahub tasks background-services command stays in #10878.

What changed

Behavioral changes:

  • The standalone task manager reads only the task_manager section of infrahub.toml and the INFRAHUB_TASK_MANAGER_* variables, plus INFRAHUB_LOG_LEVEL and INFRAHUB_PRODUCTION for its logging. An invalid value in another section no longer stops it. Distributed mode loads the whole configuration, as before.
  • Webserver-only warning: when PREFECT__SERVER_WEBSERVER_ONLY is set, the task manager warns that the process that runs Prefect's background services does not read task_manager.retention.
  • prefect_own_events longer than activity_log is refused at startup (@model_validator) instead of being capped at runtime. Prefect already caps each override by the event retention it uses, so the overrides now carry prefect_own_events as set. This reverses decision D5 (warn and cap) of the design doc.
  • An empty PREFECT_SERVER_SERVICES_DB_VACUUM_ENABLED environment variable no longer counts as set. Before, Prefect read it as false, so no task runs were deleted.
  • A preset PREFECT_SERVER_SERVICES_DB_VACUUM_ENABLED is no longer reported as overriding task_history. The task manager now warns for each cleanup that the switch leaves off (flow_runs or events) and names the retention that is not applied.
  • A retention set in Prefect's .env, prefect.toml, [tool.prefect] in pyproject.toml or the active profile now takes precedence, with a warning, like an environment variable. Before, Infrahub replaced it without a warning. The startup log shows the values Prefect parsed.
  • Per-type event overrides are written only when they shorten the event retention. With the defaults (7d and 7d) the overrides are {}, so Prefect no longer runs 103 per-type delete passes every hour.
  • Prefect event list: it adds prefect.workspace.transfer.started and prefect.workspace.transfer.completed. New unit tests scan the installed Prefect source for event names, so a Prefect upgrade that adds an event fails CI.
  • test_event_retention.py runs on a database of its own, so it no longer deletes events seeded by other component tests in the same worker.
  • The changelog fragment, the spec set and the generated configuration reference are updated to match.

Findings not fixed:

  • "Importing infrahub.config changes the standalone task manager logging" was not reproduced. The prefect.collections entry point already imported infrahub.log before #10862. With #10862 the root logger stays at Prefect's WARNING level.
  • The minor findings (duplicated logic in tasks/docs.py, schema regeneration on every call, constants copied in test_retention.py) are not in this PR.

What stayed the same: no Prefect database schema change, Infrahub events are untouched, and the setting names, defaults and variable names are unchanged.

How to review

  1. backend/infrahub/prefect_server/retention_settings.py (new: the task_manager section reader and the validator), then retention.py and app.py.
  2. Tests: backend/tests/unit/prefect_server/ and backend/tests/unit/config/test_task_manager_retention_settings.py.

Worth extra scrutiny:

  • The refusal also applies when the operator presets PREFECT_SERVER_EVENTS_RETENTION_PERIOD, even though activity_log is not in effect then.
  • The source scan in test_prefect_event_types.py finds event names written as string literals or f-strings only.

How to test

uv run pytest backend/tests/unit/config/test_task_manager_retention_settings.py backend/tests/unit/prefect_server/ -n 0
uv run pytest backend/tests/component/task_manager/test_event_retention.py -n 0

Local results: 163 unit tests pass, and ruff, mypy, ty check . and docs.validate pass. The component test was not run (no Docker in the environment), so CI is its first run.

Impact & rollout

  • Backward compatibility: a configuration with prefect_own_events longer than activity_log now stops the task manager. Lowering activity_log below 7 days needs prefect_own_events set no longer than it.
  • Performance: with the defaults, Prefect runs 103 fewer delete passes every hour.
  • Deployment notes: when the background services run in their own process, that process must run infrahub tasks background-services (#10878). The Helm chart also needs opsmill/infrahub-helm#100.

Stack impact

PRs 2 to 6 need a rebase after this lands:

  • #10869: task_history.py reads the retention period without checking that flow_runs is in the vacuum switch. The cleanup command should refuse, or say that deletion is off, when it is not.
  • #10877: the app.py import line and the lifespan setup conflict. The startup check for unlisted event types should skip when no override is in effect. The integration test assumes overrides under the 7d/7d defaults.
  • #10878: apply_infrahub_settings_to_prefect takes a config file path. The test that expects P7D for <count> event types now gets {}.
  • #10879: the precedence section in tasks.mdx, and activity-log.mdx ("capped with a warning" is now a refusal at startup).

Suggested change to the description of #10862: "Ship with #10869 and #10878 in the same release", because without #10878 a separate background-services process deletes no task runs.

Checklist

  • Tests added/updated
  • Changelog entry added
  • External docs updated (#10879 needs the updates listed above)
  • Internal .md docs updated (spec set)
  • I have reviewed AI generated content

Generated by Claude Code