codeant-ai-for-open-source[bot] commented on code in PR #41075:
URL: https://github.com/apache/superset/pull/41075#discussion_r3574638822
##########
superset/config.py:
##########
@@ -1575,6 +1575,36 @@ def sync_theme_logo_href(
os.environ.get("ENABLE_VERSIONING_CAPTURE", "false")
)
+# Retention window (days) for entity version history. Version rows
+# whose owning ``version_transaction.issued_at`` is older than this
+# value are pruned by the ``version_history.prune_old_versions``
+# Celery beat task (registered below in ``CeleryConfig.beat_schedule``).
+# If any row anchored at a transaction is live
+# (``end_transaction_id IS NULL``), that entire transaction is preserved.
+# Baseline rows (``operation_type=0``) and closed historical rows otherwise
+# age out alongside the rest. Any non-positive value disables pruning.
+# Read from environment variable of the same name.
+_DEFAULT_VERSION_HISTORY_RETENTION_DAYS: int = 30
+
+
+def _parse_version_history_retention_days() -> int:
+ """Parse the retention window without making invalid input fatal."""
+ value = os.environ.get("SUPERSET_VERSION_HISTORY_RETENTION_DAYS")
+ if value is None:
+ return _DEFAULT_VERSION_HISTORY_RETENTION_DAYS
+ try:
+ return int(value)
+ except ValueError:
Review Comment:
**Suggestion:** The retention parser accepts any integer value, but
downstream pruning computes a `timedelta(days=retention_days)` which overflows
above 999,999,999 days. This means a too-large env value is accepted at startup
and then causes the Celery task to fail every run. Treat out-of-range values as
invalid (log and fall back to default, or clamp to a safe max) when parsing.
[logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Version-history prune task crashes for oversized retention values.
- ⚠️ Shadow version tables never pruned, disk usage grows.
- ⚠️ Operators see nightly task failures in Celery logs.
- ⚠️ Misconfigured env var silently accepted instead of rejected.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure the environment variable
`SUPERSET_VERSION_HISTORY_RETENTION_DAYS` to a very
large integer, for example `1000000000`, which is read in
`superset/config.py:41-54` by
`_parse_version_history_retention_days()` using `value =
os.environ.get("SUPERSET_VERSION_HISTORY_RETENTION_DAYS")` and `return
int(value)` with no
range checks.
2. Start the Superset web application and Celery workers so that Flask loads
`SUPERSET_VERSION_HISTORY_RETENTION_DAYS` from `superset/config.py:57` into
`current_app.config` without errors, because
`_parse_version_history_retention_days()`
only catches `ValueError` for non-integer input and accepts all integer
magnitudes.
3. Wait for the daily Celery beat schedule entry
`"version_history.prune_old_versions"`
(registered in `CeleryConfig.beat_schedule` in
`superset/config.py:1670-1686`) to fire, or
manually invoke the `version_history.prune_old_versions` task defined in
`superset/tasks/version_history_retention.py:10-21`, which reads
`retention_days =
int(current_app.config.get("SUPERSET_VERSION_HISTORY_RETENTION_DAYS", 30))`
and passes
this large value to `_prune_old_versions_impl(retention_days)`.
4. Observe that `_prune_old_versions_impl` in
`superset/tasks/version_history_retention.py:452-495` computes `cutoff =
datetime.now(timezone.utc).replace(tzinfo=None) -
timedelta(days=retention_days)`, and
with `retention_days` set to a huge value this subtraction raises an
`OverflowError`,
causing the outer `prune_old_versions` wrapper (lines 17-28) to log
`logger.exception("version_history.prune_old_versions: task failed")`,
increment the
`{_METRIC_PREFIX}.failed` counter, and return `{"error": 1}`, so the
retention job fails
on every run while old version rows are never pruned.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d37198f173e14d43a4ad650d5429eaa1&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=d37198f173e14d43a4ad650d5429eaa1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/config.py
**Line:** 1595:1597
**Comment:**
*Logic Error: The retention parser accepts any integer value, but
downstream pruning computes a `timedelta(days=retention_days)` which overflows
above 999,999,999 days. This means a too-large env value is accepted at startup
and then causes the Celery task to fail every run. Treat out-of-range values as
invalid (log and fall back to default, or clamp to a safe max) when parsing.
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%2F41075&comment_hash=7af437a20045160c55ac49689f97c327d40da044299a65cd7c90ba1424b7b228&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41075&comment_hash=7af437a20045160c55ac49689f97c327d40da044299a65cd7c90ba1424b7b228&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]
