bito-code-review[bot] commented on code in PR #44514:
URL: https://github.com/apache/superset/pull/44514#discussion_r4098577148
##########
docs/docs/using-superset/version-history.mdx:
##########
@@ -30,15 +30,59 @@ FEATURE_FLAGS = {"VERSION_HISTORY": False}
ENABLE_VERSIONING_CAPTURE = False
```
-Restart Superset and its workers for the capture change to take effect.
Existing
-history remains readable while capture is off, but **Restore** is unavailable
-(404).
+Restart Superset and its workers for the capture change to take effect.
+With `VERSION_HISTORY` off, the version-list, version-snapshot, activity, and
+version-restore endpoints return 404 for callers who pass the existing route
+permissions. The flag does not grant access: when enabled, route permissions
+and object-level editorship are still required.
+
+Disabling only capture leaves existing history readable when `VERSION_HISTORY`
+is enabled, but **Restore** is unavailable (404). Disabling only
+`VERSION_HISTORY` does not stop capture or retention, and does not change
+ordinary chart, dashboard, or dataset CRUD or soft-delete recovery.
+
+Hosts can separately configure `VERSIONING_CAPTURE_PREDICATE`, a callable
+receiving the SQLAlchemy session. Its default, `None`, preserves capture
behavior.
+A false decision skips baseline, shadow, association-history, and change-record
+writes while ordinary ORM saves still persist. Version restore returns 404 when
+capture is denied. The startup `ENABLE_VERSIONING_CAPTURE` kill switch still
wins.
+The host must keep decisions tenant-local and stable for a transaction, handle
+expected service outages, and bound any cache to the transaction/request. This
+hook does not govern retention or delete existing history.
+
+Host integrations can require
`superset.versioning.utils.HOST_POLICY_API_VERSION`
+equal to `1` before installing policies. This contract includes the version API
+feature gates, transaction-scoped capture and restore, canonical retention key,
+and authoritative soft-delete retention callback. A downstream host must adopt
a
+core revision providing the entire contract, not only configure a UI flag.
+
+Imports and background ORM writes use the same predicate; hosts must supply
their
+tenant context there too. Bulk SQL retains its existing capture limitations.
+After re-enabling, skipped edits are not reconstructed. If an entity has no
+history, its first enabled edit records the existing pre-edit baseline of its
+then-current state; an entity with history resumes with its next captured
change.
Disable them together: capture off with the UI left on gives a panel that
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.
+## Retention
+
+History retention is configured independently with
`VERSION_HISTORY_RETENTION_DAYS`
+in `superset_config.py` or the environment variable of the same name. Its
default
+is 30 days; set it to `0` to disable pruning. The scheduled retention task
preserves
+live versions and prunes eligible closed history older than the configured
window.
+Changing retention does not enable history access or capture.
+
+Archived entity cleanup is separate: `SOFT_DELETE_RETENTION_DAYS` defaults to
+30 days and accepts the same-name environment seed (0 through 36500; 0 disables
+scheduled purge). A host can install `SOFT_DELETE_RETENTION_DAYS_FUNC` to
supply
+an authoritative integer window before the stored CLI override. Invalid or
+unavailable callback results defer purge with 0; they do not use stored values.
+Without that callback, the stored CLI window still takes precedence over
config.
+This does not change explicit force-purge or supply downgrade grace protection.
Review Comment:
<!-- Bito Reply -->
The reviewer's point is that the documentation mentions 'downgrade grace
protection' in the context of archived entity cleanup, but this feature is not
implemented in the deletion-retention codebase. Since the documentation could
be misleading by implying the existence of a feature that is not present,
removing the clause is recommended to ensure the documentation accurately
reflects the current implementation.
--
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]
