codeant-ai-for-open-source[bot] commented on code in PR #44514:
URL: https://github.com/apache/superset/pull/44514#discussion_r4085377765
##########
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:
**Suggestion:** The config fallback accepts values above the supported
maximum, so a huge configured window reaches `timedelta` and can crash the
cleanup task with `OverflowError`.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Possible bug`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=73511690577347d1ad34be89caa9467d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=73511690577347d1ad34be89caa9467d&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/deletion_retention/window.py
**Line:** 43:43
**Comment:**
*Possible Bug: The config fallback accepts values above the supported
maximum, so a huge configured window reaches `timedelta` and can crash the
cleanup task with `OverflowError`.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44514&comment_hash=de9a16e758bb0ebf4e813bc178c39f29252b516080abeb3d3219af54a68cdb28&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44514&comment_hash=de9a16e758bb0ebf4e813bc178c39f29252b516080abeb3d3219af54a68cdb28&reaction=dislike'>๐</a>
##########
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:
**Suggestion:** Persisted values above the supported maximum are returned
unchanged, so an oversized CLI setting can make cleanup fail during cutoff
calculation.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Possible bug`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=1f32dd8efd554fc7b79309d863915dbb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=1f32dd8efd554fc7b79309d863915dbb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/deletion_retention/window.py
**Line:** 91:91
**Comment:**
*Possible Bug: Persisted values above the supported maximum are
returned unchanged, so an oversized CLI setting can make cleanup fail during
cutoff calculation.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44514&comment_hash=7833e2e17de9784720c648d2d3b890b4a664d43e1529ef42e398d95e08c52909&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44514&comment_hash=7833e2e17de9784720c648d2d3b890b4a664d43e1529ef42e398d95e08c52909&reaction=dislike'>๐</a>
--
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]
