Abdulrehman-PIAIC80387 opened a new pull request, #41051: URL: https://github.com/apache/superset/pull/41051
### SUMMARY Fixes #40996. Reports with multiple dashboard tabs fail on the **first** generation because the per-tab permalink row is flushed-but-not-committed when Playwright tries to open it. **Root cause** (verified by [@dosu](https://github.com/apache/superset/issues/40996#issuecomment-...)): The report execution flow runs inside an outer `@transaction` block (`ReportScheduleStateMachine.run`). When `_get_tab_url` calls `CreateDashboardPermalinkCommand(...).run()`, the inner command's own `@transaction` decorator detects the active outer transaction (`g.in_transaction = True`) and skips its own commit. The new permalink row is flushed to the session but never committed to the database. Playwright then opens the permalink URL on a **separate database connection** to render the dashboard for the screenshot. Because the row isn't committed, that connection can't see it → 404 → report generation fails on the first attempt. The second attempt succeeds because the row is eventually committed later in the cycle. ### Fix Add an explicit `db.session.commit()` right after `CreateDashboardPermalinkCommand(...).run()` in `_get_tab_url`, so the permalink is visible across connections before Playwright navigates to it. This is the fix the reporter (@dijoux) tested in their production setup. The change is a single line of code (plus a multi-line comment explaining the transaction nesting trap) inside `superset/commands/report/execute.py`. ### Alternative considered `@dosu` raised one nuance: `db.session.commit()` commits **all** pending session state, not only the permalink row. At this point in `_get_tab_url` no other writes are expected — the report flow only mutates state inside the `CreateDashboardPermalinkCommand` invocation that just returned — so the commit is in practice scoped to the permalink. A more surgical SAVEPOINT-based approach is possible if reviewers prefer; happy to amend. ### TESTING INSTRUCTIONS ```bash pytest tests/unit_tests/commands/report/execute_test.py -k "tab_url" -v ``` The existing `test_get_tab_url` and `test_get_tab_urls` tests both mock `CreateDashboardPermalinkCommand.run`, so the added `db.session.commit()` runs against an empty session and is a no-op for those tests — they should continue to pass without modification. End-to-end verification: configure a scheduled dashboard report with `ALERT_REPORT_TABS=True` and multiple tab anchors. Before this change the first run produces a Playwright 404 on the permalink. After this change the first run succeeds. ### ADDITIONAL INFORMATION - [x] Has associated issue: #40996 - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (please mention any potential downtime) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [x] Runtime introduces no new dependencies / changes existing dependencies - [ ] Introduces new feature or API - [ ] 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]
