mikebridge commented on code in PR #44514:
URL: https://github.com/apache/superset/pull/44514#discussion_r4124341294
##########
superset/config.py:
##########
@@ -1794,31 +1826,59 @@ class ExportStorageConfig(TypedDict, total=False):
def _parse_version_history_retention_days() -> int:
"""Parse the retention window without making invalid input fatal."""
- value: str | None =
os.environ.get("SUPERSET_VERSION_HISTORY_RETENTION_DAYS")
+ value: str | None = os.environ.get("VERSION_HISTORY_RETENTION_DAYS")
+ legacy: bool = False
+ if value is None:
+ value = os.environ.get("SUPERSET_VERSION_HISTORY_RETENTION_DAYS")
+ legacy = value is not None
if value is None:
return _DEFAULT_VERSION_HISTORY_RETENTION_DAYS
+ return _normalize_version_history_retention_days(value, legacy=legacy)
+
+
+def _normalize_version_history_retention_days(value: object, *, legacy: bool)
-> int:
+ """Normalize released legacy values without shortening retention on
upgrade."""
+ name: str = (
+ "SUPERSET_VERSION_HISTORY_RETENTION_DAYS"
+ if legacy
+ else "VERSION_HISTORY_RETENTION_DAYS"
+ )
try:
- retention_days = int(value)
+ if isinstance(value, bool) or not isinstance(value, (str, int)):
+ raise ValueError("Retention must be integer days")
+ retention_days: int = int(value)
except ValueError:
+ logger.warning("Invalid %s=%r; skipping pruning", name, value)
+ return 0
+ if legacy:
logger.warning(
- "Invalid SUPERSET_VERSION_HISTORY_RETENTION_DAYS=%r; using %d",
- value,
- _DEFAULT_VERSION_HISTORY_RETENTION_DAYS,
+ "%s is deprecated; use VERSION_HISTORY_RETENTION_DAYS. "
+ "Legacy nonpositive values disable pruning.",
+ name,
)
- return _DEFAULT_VERSION_HISTORY_RETENTION_DAYS
+ if retention_days <= 0:
+ return 0
+ if retention_days < -1:
+ logger.warning("Invalid negative %s; skipping pruning", name)
+ return 0
if retention_days > _MAX_VERSION_HISTORY_RETENTION_DAYS:
logger.warning(
- "SUPERSET_VERSION_HISTORY_RETENTION_DAYS=%r exceeds the maximum "
- "of %d; using %d",
+ "%s=%r exceeds the maximum of %d; skipping pruning",
+ name,
value,
_MAX_VERSION_HISTORY_RETENTION_DAYS,
- _DEFAULT_VERSION_HISTORY_RETENTION_DAYS,
)
- return _DEFAULT_VERSION_HISTORY_RETENTION_DAYS
+ return 0
+ if retention_days == -1:
+ logger.warning(
+ "VERSION_HISTORY_RETENTION_DAYS=-1 makes history eligible for "
+ "immediate pruning on the next scheduled run; use 0 to disable"
+ )
Review Comment:
Updated in `903a799ae3a8`: the warning now uses the setting name
consistently, with regression coverage. The legacy `-1` path already returns
`0` before this warning; the regression confirms it still disables pruning and
emits no immediate-pruning warning.
##########
superset/config.py:
##########
@@ -3545,6 +3639,28 @@ def _config_fingerprint(source: bytes | None) -> str:
logger.exception("Found but failed to import local superset_config")
raise
+if _canonical_history_retention_override:
+ VERSION_HISTORY_RETENTION_DAYS = _normalize_version_history_retention_days(
+ _canonical_history_retention_value, legacy=False
+ )
+if (
+ _legacy_history_retention_override
+ and "VERSION_HISTORY_RETENTION_DAYS" not in os.environ
+):
+ _legacy_history_retention_days: int =
_normalize_version_history_retention_days(
+ _legacy_history_retention_value, legacy=True
+ )
+ if not _canonical_history_retention_override:
+ VERSION_HISTORY_RETENTION_DAYS = _legacy_history_retention_days
+ elif VERSION_HISTORY_RETENTION_DAYS == _version_history_retention_seed:
Review Comment:
Addressed in `903a799ae3a8`. The consolidated resolver already warned for a
valid legacy config value before a differing canonical override won; a
config-load regression now covers that case. Also fixed the remaining
ignored-legacy paths: a canonical environment setting still allows the legacy
deprecation warning, and ignored invalid legacy values no longer claim active
pruning is skipped. Retention results and precedence are unchanged; 581
affected tests passed, and a differential probe matched the previous returned
values in 608 cases.
--
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]
