eschutho opened a new pull request, #41080:
URL: https://github.com/apache/superset/pull/41080
### SUMMARY
Two related changes to `superset/utils/webdriver.py`:
**1. Require `user` in `get_screenshot` / simplify `WebDriverSelenium`
lifecycle**
`get_screenshot` previously accepted `User | None` with defensive guards
throughout. All callers pass a real `User`, so the optional was dead code that
obscured the type contract and caused username logging to emit `"None"` instead
of raising early.
`WebDriverSelenium` had a persistent-driver pattern (`__init__` storing
`_user`/`_driver`, a lazy `driver` property, public `create()`/`destroy()`
wrappers). This made the class stateful in a way that risked reusing a stale
driver across requests. The class is now stateless: `get_screenshot` creates
the driver, authenticates, takes the screenshot, and destroys the driver in a
`finally` block. `screenshots.py` is updated to match (drop the now-redundant
external `destroy()` call and the unused `user` arg to the constructor).
**2. Fail instead of silently falling back on tiled screenshot error**
When `take_tiled_screenshot` returned `None`, the previous code immediately
fell back to a standard `element.screenshot()` with **no spinner wait and no
animation wait**. This produced a blank (all-white) image that became a blank
PDF silently sent to report recipients.
The fallback is removed. Returning `None` now propagates through
`_get_screenshots()` → `ReportScheduleScreenshotFailedError` → ERROR execution
state with owner notification, so failures are visible rather than silent.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend change, no UI impact.
### TESTING INSTRUCTIONS
1. Run the new unit test:
```bash
pytest tests/unit_tests/utils/webdriver_test.py -k
"test_tiled_screenshot_failure_returns_none_without_fallback" -v
```
2. To verify the blank-PDF fix end-to-end: configure a report on a large
dashboard (≥20 charts or >5000px height, so tiled mode activates) with
`SCREENSHOT_TILED_ENABLED=True`, then trigger a condition where
`take_tiled_screenshot` fails (e.g., element not found). Confirm the execution
state is ERROR and the owner receives an error notification rather than a blank
PDF.
3. To verify the `User` requirement: mypy passes (`pre-commit run mypy`).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
Related to #39895 and #40694 (per-tile spinner and animation waits).
--
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]
