codeant-ai-for-open-source[bot] commented on code in PR #43553:
URL: https://github.com/apache/superset/pull/43553#discussion_r3863099780
##########
superset/reports/schemas.py:
##########
@@ -180,7 +189,31 @@ def validate_addresses(field: str, value: str | None,
required: bool) -> None:
validate_addresses("bccTarget", config.get("bccTarget"),
required=False)
-class ReportSchedulePostSchema(Schema):
+_RETRY_FIELD_KEYS = (
+ "retry_on_failure",
+ "retry_max_attempts",
+ "send_failed_reports",
+ "retry_notify_owners",
+ "retry_notify_recipients",
+)
+
+
+class RetryFieldStripMixin:
+ """Strip retry fields from the deserialized payload when the feature is
off."""
+
+ @post_load
+ def strip_retry_fields_if_disabled(
+ self,
+ data: dict[str, Any],
+ **kwargs: Any,
+ ) -> dict[str, Any]:
+ if not is_feature_enabled("ALERT_REPORTS_RETRY"):
+ for key in _RETRY_FIELD_KEYS:
+ data.pop(key, None)
+ return data
Review Comment:
**Suggestion:** The retry fields are removed in a `post_load` hook, but
Marshmallow runs field and schema validation before `post_load`. With
`ALERT_REPORTS_RETRY` disabled, payloads containing values such as
`retry_max_attempts=0` or `retry_max_attempts=11` still fail validation instead
of being ignored and stripped as intended. Strip or bypass these fields before
validation, or conditionally disable their validators when the feature flag is
off. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Report schedule POST/PUT writes reject disabled retry fields.
- ⚠️ External API clients cannot rely on disabled fields being ignored.
- ⚠️ Feature-flag rollout can produce unexpected validation failures.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3fede4fd705444c18366872c9e4a9fd6&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=3fede4fd705444c18366872c9e4a9fd6&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/reports/schemas.py
**Line:** 201:213
**Comment:**
*Api Mismatch: The retry fields are removed in a `post_load` hook, but
Marshmallow runs field and schema validation before `post_load`. With
`ALERT_REPORTS_RETRY` disabled, payloads containing values such as
`retry_max_attempts=0` or `retry_max_attempts=11` still fail validation instead
of being ignored and stripped as intended. Strip or bypass these fields before
validation, or conditionally disable their validators when the feature flag is
off.
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%2F43553&comment_hash=d4dfb6e1df9d6b3714197170287d0c80f633465ac8de916c074b563815f9ba78&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43553&comment_hash=d4dfb6e1df9d6b3714197170287d0c80f633465ac8de916c074b563815f9ba78&reaction=dislike'>👎</a>
##########
superset/commands/report/execute.py:
##########
@@ -1767,7 +1771,8 @@ def next(self) -> None: # noqa: C901
return
self.send()
# Clear any retry state from previous failed attempts in this
window.
- self._reset_retry_counter()
+ if feature_flag_manager.is_feature_enabled("ALERT_REPORTS_RETRY"):
+ self._reset_retry_counter()
Review Comment:
**Suggestion:** When the feature flag is disabled after a schedule has
entered `RETRYING`, successful executions no longer clear `retry_attempt` or
`retry_scheduled_dttm`. The schedule can therefore retain exhausted retry state
while retries are disabled, and the stale counter may be reused when the flag
is later enabled, causing the first subsequent failure to skip retries. Clear
stale retry state whenever execution reaches a terminal success, regardless of
whether retry handling is currently enabled. [stale reference]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Re-enabled schedules can skip their first retry.
- ⚠️ Runtime feature-flag changes leave inconsistent retry state.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=29e07ea6c6194040964bef6f4de1fd14&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=29e07ea6c6194040964bef6f4de1fd14&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/report/execute.py
**Line:** 1774:1775
**Comment:**
*Stale Reference: When the feature flag is disabled after a schedule
has entered `RETRYING`, successful executions no longer clear `retry_attempt`
or `retry_scheduled_dttm`. The schedule can therefore retain exhausted retry
state while retries are disabled, and the stale counter may be reused when the
flag is later enabled, causing the first subsequent failure to skip retries.
Clear stale retry state whenever execution reaches a terminal success,
regardless of whether retry handling is currently enabled.
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%2F43553&comment_hash=d9f2dc672f9cc8a0be63e57ff89986904b8ff42c3da9bf5b8ef6ed337720fb52&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43553&comment_hash=d9f2dc672f9cc8a0be63e57ff89986904b8ff42c3da9bf5b8ef6ed337720fb52&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]
