fitzee opened a new pull request, #44336:
URL: https://github.com/apache/superset/pull/44336
### SUMMARY
TL;DR: Close the remaining capture-validation and whole-execution retry gaps
for scheduled reports and alerts. Enable alert retry configuration, validate
final image/PDF provenance, and fence duplicate or obsolete execution attempts
before delivery.
The earlier changes are already on `master`: #43784 (capture readiness),
#43977 (CSV transport hardening), #44034 (perceptual blank detection), #44114
(capture-time readiness), #44191 (sticky capture rejection), #42481 (report
retries), and #43553 (retry feature flag). This PR builds on those changes; it
does not reintroduce legacy browser drivers or replace newer
notification/access-control behavior.
Missing safeguards addressed here:
- Alerts can opt into retries through both API and UI. A retry re-evaluates
the alert condition and clears retry state without sending if the condition no
longer fires.
- Final screenshot bytes are decoded and validated independently of capture
provenance. PDF delivery requires the exact bytes approved during assembly from
validated screenshots. Capture failures remain sticky for the execution.
- Blank detection covers neighbouring light-grey backgrounds while retaining
readable sparse content and solid-colour visualizations. This remains a
heuristic, not proof that every chart is correct.
- An atomic database claim ties execution to a schedule/window/owner.
Duplicate, obsolete, disabled and superseded retries are refused; a fenced
worker cannot proceed to delivery. Retry tasks retain timeout options and
original window identity.
- Whole-execution retry is refused once notification delivery starts,
avoiding replay of potentially accepted sends. Exhaustion sends only the
optional final retry notice, rather than an extra per-retry notice as well.
Retry notifications omit raw provider diagnostics.
- Readiness diagnostics distinguish rendered, empty, error, virtualized and
unready holders. Terminal chart errors remain capture-ready but emit a
semantic-status warning rather than being reported as successful chart data.
The change spans the validator, delivery boundary, scheduler, configuration
UI and ownership migration because these jointly enforce the same execution
contract. The larger diff includes regression tests; splitting the runtime
safeguards would leave intermediate bypasses.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Alerts gain the existing **Error handling / Enable Retries** panel
previously limited to reports. No other visual redesign.
### TESTING INSTRUCTIONS
Local automated validation:
- 876 backend tests passed across report commands, schemas/notifications,
screenshot/readiness utilities and scheduler tests.
- 99 frontend tests passed in `AlertReportModal` and `ReportModal`, using
Node 24 and the checked-in dependency lockfile.
- SQLite migration upgrade/downgrade/re-upgrade and interrupted-upgrade
cases passed. Independent database sessions test competing execution claims.
- Real PNG/PDF fixtures cover blank, sparse, solid-colour and undecodable
captures, exact-PDF provenance, rejection before conversion, and a fenced
worker being refused delivery.
- Changed-file pre-commit hooks, including Python/frontend type checks,
formatting and linting, were run locally before publishing.
Manual canary:
1. Run `superset db upgrade`, enable `ALERT_REPORTS_RETRY`, and opt an alert
and report into retries. Use a dashboard with tables/pivots and rendered
PDF/PNG attachments.
2. Induce an ordinary pre-delivery capture failure. Confirm the execution
transitions to `Retrying`, the next attempt uses the same schedule window, and
no rejected attachment is sent.
3. Change the alert condition to false before its retry. Confirm a new query
runs and the retry completes without notifying recipients.
4. Replay a retry message, disable retry opt-in before execution, or replace
its owner/window. Confirm it does not capture or send.
5. Exercise a partial/ambiguous delivery failure. Confirm it is not replayed
as a whole execution. Check that exhausted retry notices contain no rendered
attachment or raw provider error.
6. Compare near-white captures with sparse KPI/no-data output. Inspect
`report_capture_validation`, `report_delivery_blocked`, holder-state counts and
retry-admission logs.
Limitations: this is not exactly-once delivery, guaranteed recovery from
worker termination, or a guarantee that all bad chart output can be detected.
Live scheduled-report canary testing and PostgreSQL/MySQL migration
verification are still required before rollout.
### ADDITIONAL INFORMATION
- [ ] Has associated issue
- [x] Required feature flags: `ALERT_REPORTS_RETRY`; existing attachment
flags still apply
- [x] Changes UI
- [x] Includes DB Migration (SIP-59 review required)
- [ ] Migration is atomic on every supported database
- [x] Migration upgrade/downgrade tested on SQLite, including a partially
applied upgrade
- Adds two nullable columns to `report_schedule`; no schedule data
rewrite. Actual DDL duration and locking depend on metadata database/version
and must be measured for the deployment.
- Pause scheduling and drain in-flight executions and queued retries
before upgrading workers together. Old workers do not participate in ownership
fencing. Old-signature retry tasks are discarded rather than replayed without
an owner token. Rollback also requires draining workers before downgrading
these columns.
- [x] Introduces new feature or API: alert retry opt-in using existing retry
fields
- [ ] Removes existing feature or API
--
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]
