codeant-ai-for-open-source[bot] commented on code in PR #44514:
URL: https://github.com/apache/superset/pull/44514#discussion_r4087366712
##########
superset/commands/deletion_retention/window.py:
##########
@@ -36,10 +37,10 @@ def _config_retention_days() -> int:
"SOFT_DELETE_RETENTION_DAYS", _DEFAULT_RETENTION_DAYS
)
try:
- if isinstance(configured, bool):
+ if isinstance(configured, bool) or not isinstance(configured, (str,
int)):
raise ValueError
days = int(configured)
- if days < 0:
+ if days < -1:
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `edfe588`.
The config fallback now checks values against MAX_RETENTION_DAYS and returns
0 with a warning for oversized settings before they can reach timedelta.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
##########
superset/commands/deletion_retention/window.py:
##########
@@ -57,20 +58,37 @@ def resolve_retention_window() -> int:
Resolution order:
- 1. The per-deployment value persisted under
+ 1. An installed ``SOFT_DELETE_RETENTION_DAYS_FUNC`` host policy. Its
+ result is authoritative; invalid/unavailable policy defers with zero.
+ 2. The per-deployment value persisted under
``SharedKey.SOFT_DELETE_RETENTION_DAYS`` (read live; takes
precedence when present).
- 2. Otherwise the ``SOFT_DELETE_RETENTION_DAYS`` config /
+ 3. Otherwise the ``SOFT_DELETE_RETENTION_DAYS`` config /
environment seed default (itself defaulting to 30).
- ``0`` from either source is a meaningful "disable", so the shared
+ ``0`` from any source is a meaningful "disable", so the shared
value is selected with an explicit ``is None`` check — never ``or``,
which would treat ``0`` as unset. A malformed shared value is
rejected (logged) and the fallback is used rather than crashing the
scheduled task.
"""
+ policy: Callable[[], object] | None = current_app.config.get(
+ "SOFT_DELETE_RETENTION_DAYS_FUNC"
+ )
+ if policy is not None:
+ try:
+ days: object = policy()
+ except Exception: # Host boundary: do not expose service payloads or
purge.
+ logger.warning(
+ "deletion_retention: host retention policy unavailable;
skipping"
+ )
+ return 0
+ if isinstance(days, int) and not isinstance(days, bool) and -1 <= days
<= 36500:
+ return days
+ logger.warning("deletion_retention: invalid host retention policy;
skipping")
+ return 0
if (shared := get_shared_value(SharedKey.SOFT_DELETE_RETENTION_DAYS)) is
not None:
- if isinstance(shared, bool) or not isinstance(shared, int) or shared <
0:
+ if isinstance(shared, bool) or not isinstance(shared, int) or shared <
-1:
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and
marked this thread resolved** as of `edfe588`.
Persisted shared values above MAX_RETENTION_DAYS now log a warning and
return 0 instead of being returned unchanged.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it
open.</sub>
<!-- codeant-auto-resolve-reply -->
--
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]
