codeant-ai-for-open-source[bot] commented on code in PR #41075:
URL: https://github.com/apache/superset/pull/41075#discussion_r3537991925
##########
superset/config.py:
##########
@@ -1503,6 +1507,33 @@ def sync_theme_logo_href(
# The limit for the Superset Meta DB when the feature flag
ENABLE_SUPERSET_META_DB is on
SUPERSET_META_DB_LIMIT: int | None = 1000
+# Master switch for entity-version-history capture. Ships defaulted ``False``
+# so the versioning infrastructure (schema + Continuum wiring) lands inert:
+# no save writes shadow rows or a ``version_transaction``/``version_changes``
+# record, while the /versions/ endpoints stay available read-only (returning
+# empty). Set to ``True`` in ``superset_config.py`` (or via the env var of the
+# same name) to enable the before-flush listeners that drive capture.
+# Capture is activated by flipping this default to on once validated in
+# production. It is an operational escape hatch — for use when a
+# versioning-induced regression needs a 30-second recovery instead of
+# revert-and-redeploy — not a feature flag, and remains as the permanent
+# kill-switch.
+ENABLE_VERSIONING_CAPTURE: bool = utils.parse_boolean_string(
+ 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``).
+# Only the live row (``end_transaction_id IS NULL``) is preserved
+# unconditionally; baseline rows (``operation_type=0``) and any
+# historical row age out alongside the rest. ``0`` disables pruning.
+# Read from environment variable of the same name.
+SUPERSET_VERSION_HISTORY_RETENTION_DAYS: int = int(
+ os.environ.get("SUPERSET_VERSION_HISTORY_RETENTION_DAYS", "30")
Review Comment:
**Suggestion:** The direct `int(...)` cast on the environment variable can
raise `ValueError` during module import when the value is malformed (for
example `"30d"` or empty), which prevents Superset from starting at all. Parse
this setting defensively (with a safe fallback/default and optional warning) so
a bad env value disables/uses default retention instead of crashing startup.
[type error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Superset Flask app fails during create_app bootstrap.
- ❌ Celery worker startup fails loading superset.config module.
- ⚠️ Version history retention cannot run or be configured.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Ensure the default config module is used so `superset/app.py:52-66` loads
`superset.config` via `app.config.from_object(config_module)` when
`SUPERSET_CONFIG` is
unset.
2. Set environment variable `SUPERSET_VERSION_HISTORY_RETENTION_DAYS` to a
malformed value
(for example `30d` or an empty string) before starting Superset.
3. Start the web application (or any process that calls
`superset.app.create_app`), which
imports `superset/config.py` and evaluates
`SUPERSET_VERSION_HISTORY_RETENTION_DAYS: int =
int(os.environ.get("SUPERSET_VERSION_HISTORY_RETENTION_DAYS", "30"))` at
lines 54-56 in
that file.
4. Observe that `int("30d")` (or `int("")`) raises `ValueError` during
module import,
causing `create_app()` in `superset/app.py:52-70` to log "Failed to create
app" and
re-raise, preventing Superset (and its Celery workers) from starting.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=8b9015cec87c4399bfd6034fd5bf9e11&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=8b9015cec87c4399bfd6034fd5bf9e11&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:** 1533:1534
**Comment:**
*Type Error: The direct `int(...)` cast on the environment variable can
raise `ValueError` during module import when the value is malformed (for
example `"30d"` or empty), which prevents Superset from starting at all. Parse
this setting defensively (with a safe fallback/default and optional warning) so
a bad env value disables/uses default retention instead of crashing startup.
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=1598314a72ddddf1b21139cc7abb8810a8f6736fdf2219e8fab6f99795cf5d43&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41075&comment_hash=1598314a72ddddf1b21139cc7abb8810a8f6736fdf2219e8fab6f99795cf5d43&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]
