codeant-ai-for-open-source[bot] commented on code in PR #42481:
URL: https://github.com/apache/superset/pull/42481#discussion_r3681727810
##########
superset/commands/report/execute.py:
##########
@@ -1195,10 +1377,26 @@ class ReportNotTriggeredErrorState(BaseReportState):
- Error
"""
- current_states = [ReportState.NOOP, ReportState.ERROR]
+ current_states = [ReportState.NOOP, ReportState.ERROR,
ReportState.RETRYING]
initial = True
def next(self) -> None: # noqa: C901
+ # If retries from a previous crontab window are still in-flight and
+ # this is a new crontab trigger, skip — let the active retry chain
+ # finish. The stale-window check uses normalized (naive) datetimes
+ # so tz-aware vs naive mismatches don't cause false positives.
+ if (
+ self._report_schedule.last_state == ReportState.RETRYING
+ and self._is_retry_window_stale()
+ ):
+ logger.info(
+ "Skipping crontab execution for report %s — retries from a "
+ "previous window are still in-flight (execution %s)",
+ self._report_schedule.id,
+ self._execution_id,
+ )
+ return
Review Comment:
**Suggestion:** This branch permanently drops a newly scheduled crontab
window when an older retry chain is active. It returns without advancing
`last_eval_dttm`, recording an execution, or enqueuing the new window; because
the scheduler continues to produce future windows, this particular window is
never retried after the old chain finishes. The new-window execution must be
deferred and re-enqueued, or otherwise recorded so it is not lost. [incomplete
implementation]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Scheduled report windows can be silently skipped.
- ❌ Alert evaluations may miss their next scheduled check.
- ⚠️ Users receive no execution log for skipped windows.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3c23da30c81b4a988efc3c60da4db602&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=3c23da30c81b4a988efc3c60da4db602&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/commands/report/execute.py
**Line:** 1388:1398
**Comment:**
*Incomplete Implementation: This branch permanently drops a newly
scheduled crontab window when an older retry chain is active. It returns
without advancing `last_eval_dttm`, recording an execution, or enqueuing the
new window; because the scheduler continues to produce future windows, this
particular window is never retried after the old chain finishes. The new-window
execution must be deferred and re-enqueued, or otherwise recorded so it is not
lost.
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%2F42481&comment_hash=cde04d64be83a84e557f972e8984cfe1bce39e9821623579bc4afb08861f78ea&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42481&comment_hash=cde04d64be83a84e557f972e8984cfe1bce39e9821623579bc4afb08861f78ea&reaction=dislike'>👎</a>
##########
superset/commands/report/execute.py:
##########
@@ -1165,6 +1165,188 @@ def is_in_error_grace_period(self) -> bool:
< last_success.end_dttm
)
+ def _get_retry_delay(self, attempt: int) -> int:
+ """Exponential backoff: base * 2^attempt, capped at a configurable
max."""
+ base: int = app.config.get("ALERT_REPORTS_RETRY_BASE_DELAY_SECONDS",
60)
+ cap: int = app.config.get("ALERT_REPORTS_RETRY_MAX_DELAY_SECONDS",
3600)
+ return min(base * (2**attempt), cap)
+
+ @staticmethod
+ def _normalize_dttm(dt: Optional[datetime]) -> Optional[datetime]:
+ """Strip timezone info so naive/aware datetimes can be compared."""
+ if dt is not None and dt.tzinfo is not None:
+ return dt.replace(tzinfo=None)
+ return dt
+
+ def _is_retry_window_stale(self) -> bool:
+ """
+ Return True if a new crontab window fired while the previous one was
+ still retrying. When stale the retry counter must be reset so the new
+ window gets a fresh retry budget.
+
+ Both sides are normalized to naive datetimes because
+ retry_scheduled_dttm is a plain DateTime column (no timezone), while
+ _scheduled_dttm may be tz-aware depending on the Celery broker.
+ """
+ anchor =
self._normalize_dttm(self._report_schedule.retry_scheduled_dttm)
+ current = self._normalize_dttm(self._scheduled_dttm)
+ return anchor is not None and anchor != current
+
+ def _increment_retry(self) -> int:
+ """Increment retry_attempt, set the window anchor, and return the new
count."""
+ self._report_schedule.retry_attempt += 1
+ # Store as naive — the column is a plain DateTime.
+ self._report_schedule.retry_scheduled_dttm = self._normalize_dttm(
+ self._scheduled_dttm
+ )
+ return self._report_schedule.retry_attempt
Review Comment:
**Suggestion:** The retry counter is modified on the ORM object and then
persisted without acquiring a row lock or performing an atomic
compare-and-update. Two duplicate Celery executions can both observe the same
`retry_attempt`, each enqueue another retry, and overwrite the retry anchor,
causing duplicate report deliveries and exceeding the configured retry budget.
[race condition]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Duplicate retry tasks can deliver duplicate reports.
- ❌ Retry budgets can be exceeded under overlapping execution.
- ⚠️ Retry state anchors can be overwritten by stale workers.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2f290fca1ee046e38014a75a53fc6b56&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=2f290fca1ee046e38014a75a53fc6b56&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/commands/report/execute.py
**Line:** 1195:1202
**Comment:**
*Race Condition: The retry counter is modified on the ORM object and
then persisted without acquiring a row lock or performing an atomic
compare-and-update. Two duplicate Celery executions can both observe the same
`retry_attempt`, each enqueue another retry, and overwrite the retry anchor,
causing duplicate report deliveries and exceeding the configured retry budget.
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%2F42481&comment_hash=39935f9cf1ca1e9625a44e6ba099e43e07d3e9c99427e1d2e37a88cab420d732&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42481&comment_hash=39935f9cf1ca1e9625a44e6ba099e43e07d3e9c99427e1d2e37a88cab420d732&reaction=dislike'>👎</a>
##########
superset/migrations/versions/2026-07-28_00-00_f3a8c1d2e9b7_add_report_retry_state_columns.py:
##########
@@ -0,0 +1,84 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""add_report_retry_columns
+
+Revision ID: f3a8c1d2e9b7
+Revises: d3b9a1f6c204
+Create Date: 2026-07-28 00:00:00.000000
+
+"""
+
+import logging
+
+import sqlalchemy as sa
+from alembic import op
+
+from superset.migrations.shared.utils import get_table_column
+
+logger = logging.getLogger("alembic.env")
+
+# revision identifiers, used by Alembic.
+revision = "f3a8c1d2e9b7"
+down_revision = "d3b9a1f6c204"
+
+# Configuration columns (user-configurable)
+_CONFIG_COLUMNS = [
+ ("retry_on_failure", sa.Boolean(), False, "0"),
+ ("retry_max_attempts", sa.Integer(), False, "3"),
+ ("send_failed_reports", sa.Boolean(), False, "0"),
+ ("retry_notify_owners", sa.Boolean(), False, "1"),
+ ("retry_notify_recipients", sa.Boolean(), False, "0"),
+]
+
+# State columns (written by the execution engine)
+_STATE_COLUMNS = [
+ ("retry_attempt", sa.Integer(), False, "0"),
+ ("retry_scheduled_dttm", sa.DateTime(), True, None),
+]
+
+
+def upgrade() -> None:
+ """Add retry config and state columns to report_schedule."""
+ if get_table_column("report_schedule", "retry_attempt") is not None:
+ logger.info(
+ "Column report_schedule.retry_attempt already exists. Skipping
migration."
+ )
+ return
Review Comment:
**Suggestion:** Using only `retry_attempt` as the migration-completion
marker makes the migration non-recoverable after a partial DDL application. If
that column was created but any later retry column was not, a rerun returns
early and leaves the model querying nonexistent columns. Check that every
expected column exists, or add missing columns individually. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Failed upgrades can leave report execution unusable.
- ❌ Retry configuration fields may be missing after migration reruns.
- ⚠️ Operators must manually repair partially applied schemas.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=07d912c711ab476fab8c68c980cce43d&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=07d912c711ab476fab8c68c980cce43d&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/migrations/versions/2026-07-28_00-00_f3a8c1d2e9b7_add_report_retry_state_columns.py
**Line:** 56:60
**Comment:**
*Possible Bug: Using only `retry_attempt` as the migration-completion
marker makes the migration non-recoverable after a partial DDL application. If
that column was created but any later retry column was not, a rerun returns
early and leaves the model querying nonexistent columns. Check that every
expected column exists, or add missing columns individually.
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%2F42481&comment_hash=6b266d770ed29f8db49be66f8401afb93024c4a0b006f21f0889edf4f2dadfb2&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42481&comment_hash=6b266d770ed29f8db49be66f8401afb93024c4a0b006f21f0889edf4f2dadfb2&reaction=dislike'>👎</a>
##########
superset/commands/report/execute.py:
##########
@@ -1403,10 +1606,15 @@ def next(self) -> None:
warning_message = (
";".join(self._filter_warnings) if self._filter_warnings else
None
)
+ # Clear any retry state from previous failed attempts in this
window.
+ self._reset_retry_counter()
self.update_report_schedule_and_log(
ReportState.SUCCESS, error_message=warning_message
)
except Exception as ex: # pylint: disable=broad-except
+ if self._handle_retry_or_error(str(ex), ex):
+ return # retry scheduled — exit cleanly
Review Comment:
**Suggestion:** The retry handler is invoked only from the report send
failure path shown here. For alerts that are already in `SUCCESS` or `GRACE`,
failures from the preceding `AlertCommand.run()` are handled by the separate
alert exception branch and bypass `_handle_retry_or_error`, so configured
retries do not apply to those alert evaluations. Route that alert failure path
through the same retry logic. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Configured retries do not cover alert evaluation failures.
- ❌ Transient alert query failures immediately enter ERROR.
- ⚠️ Alert owners receive failure notifications without retry attempts.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f8578cbf23c44d808b2c78f00985a8ae&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=f8578cbf23c44d808b2c78f00985a8ae&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/commands/report/execute.py
**Line:** 1615:1616
**Comment:**
*Incomplete Implementation: The retry handler is invoked only from the
report send failure path shown here. For alerts that are already in `SUCCESS`
or `GRACE`, failures from the preceding `AlertCommand.run()` are handled by the
separate alert exception branch and bypass `_handle_retry_or_error`, so
configured retries do not apply to those alert evaluations. Route that alert
failure path through the same retry logic.
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%2F42481&comment_hash=25c14c84d44c460dd8687439f95a53807289d0ee0c4d98f6aa7b985de84d6d58&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42481&comment_hash=25c14c84d44c460dd8687439f95a53807289d0ee0c4d98f6aa7b985de84d6d58&reaction=dislike'>👎</a>
##########
superset/reports/api.py:
##########
@@ -147,6 +147,11 @@ def ensure_alert_reports_enabled(self) ->
Optional[Response]:
"validator_type",
"working_timeout",
"email_subject",
+ "retry_on_failure",
+ "retry_max_attempts",
+ "send_failed_reports",
+ "retry_notify_owners",
+ "retry_notify_recipients",
Review Comment:
**Suggestion:** The retry fields are exposed in `show_columns`, but they are
absent from `add_columns` and `edit_columns`, so FAB filters them out of create
and update requests before persistence. The UI can submit these settings
successfully while the database retains the defaults. Add all retry fields to
the writable column configuration as well. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ UI retry settings cannot be persisted.
- ❌ Reports continue using default retry behavior.
- ⚠️ GET responses falsely suggest settings were accepted.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6429313e4c884b3c8f650cbd48934969&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=6429313e4c884b3c8f650cbd48934969&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/reports/api.py
**Line:** 150:154
**Comment:**
*Api Mismatch: The retry fields are exposed in `show_columns`, but they
are absent from `add_columns` and `edit_columns`, so FAB filters them out of
create and update requests before persistence. The UI can submit these settings
successfully while the database retains the defaults. Add all retry fields to
the writable column configuration as well.
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%2F42481&comment_hash=47b6693402f2d33275f986798ec481a50127747cccfbc2f7121f047e7be8450c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42481&comment_hash=47b6693402f2d33275f986798ec481a50127747cccfbc2f7121f047e7be8450c&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]
