gabotorresruiz commented on code in PR #44190:
URL: https://github.com/apache/superset/pull/44190#discussion_r4084956059


##########
docs/docs/using-superset/version-history.mdx:
##########
@@ -39,13 +56,47 @@ stops filling — an empty or stale history misrepresents the 
entity as
 unchanged. History only accrues while capture is on; edits made while it was
 off are not reconstructed.
 
+## How long history is kept
+
+The `version_history.prune_old_versions` background task prunes expired
+history. The default Celery beat configuration schedules it daily at 03:00
+in the configured Celery timezone. It runs only when beat and workers are
+running with the required schedule and task registration; see the
+[operator checklist](./recently-archived.mdx#running-retention-tasks).
+
+The `SUPERSET_VERSION_HISTORY_RETENTION_DAYS` configuration value defaults to
+30 days. It is initialized from the environment variable of the same name
+and can be overridden in `superset_config.py`. Restart the relevant processes
+after changing environment or configuration values.
+
+At startup, a non-integer environment value or one above 36,500 logs a warning
+and falls back to 30 days. Use zero, not an arbitrarily large number, to
+disable pruning.
+
+Pruning preserves live version rows and their anchoring transactions,
+regardless of age. It does not preserve every historical row that shares
+one of those transactions: closed rows can be removed when their closing
+transaction expires. Retention can therefore remove history used by previews
+and restores; version history is not a substitute for backups.
+
+Set the value to zero (or any non-positive value) to disable history pruning.
+There is no dry-run switch for this task. Neither `VERSION_HISTORY` nor
+`ENABLE_VERSIONING_CAPTURE` disables pruning of existing history. To preserve
+history while the feature is off, set the retention value to zero or remove
+the pruning task's beat schedule entry. This window governs version

Review Comment:
   Not a blocker, but the second option here leaves the app warning on every 
boot.
   
   `_warn_if_retention_beat_missing` warns whenever 
`version_history.prune_old_versions` is absent from `beat_schedule`, with no 
guard on the retention value or on either flag 
(`superset/initialization/__init__.py:989`), and its own docstring says that 
check runs always (`superset/initialization/__init__.py:915`). I read the whole 
method on this head: between the `CELERY_CONFIG is None` early return and that 
branch there is nothing to short circuit it. So an operator who removes the 
beat entry gets `CELERY_CONFIG.beat_schedule is missing the 
'version_history.prune_old_versions' entry ... shadow tables will grow 
unbounded` at every startup, while `recently-archived.mdx:161` in this same PR 
tells them to check startup warnings for missing schedules.
   
   The first option already does the job silently: `_prune_old_versions_impl` 
returns `{"skipped": 1}` for any non-positive value 
(`superset/tasks/version_history_retention.py:479`). Worth dropping the 
`beat_schedule` alternative here, or saying that it trips that warning 
permanently?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to