eschutho opened a new pull request, #42153: URL: https://github.com/apache/superset/pull/42153
### Summary `WebDriverPlaywright.get_screenshot()` waits for `.chart-container` elements to render before deciding whether to use tiled or standard screenshot capture for a dashboard. Two issues in that logic: 1. A timeout while waiting for chart containers to render was logged at `ERROR` (`logger.exception`), inconsistent with the `WARNING` level already used for the sibling spinner-wait timeouts a few lines below in the same method. 2. When the DOM query used to measure the dashboard's height returns `0` (the query found no matching element, or the layout hadn't settled yet), the code treated that the same as "the dashboard is small enough for a single tile" — which could veto tiled capture even when the chart count alone indicated a large dashboard. That could let a large, virtualized dashboard skip the scroll-driven tiling pass and ship with below-the-fold charts that never rendered. ### Fix - The chart-container timeout now logs at `WARNING`, matching the convention already used for the other Playwright locate-wait timeouts in this method (customer-side chart loading is usually just slow, not a Superset bug). The screenshot still fails — the exception is still re-raised — only the log level and message changed; the message now also reports how many chart containers rendered before the timeout, for easier debugging. - An unmeasurable height (`0`) is now treated as "unknown" rather than "small". Whether tiling is attempted is decided by the chart-count signal alone when height can't be measured, instead of chart count being silently overridden. The corresponding log line is only emitted at `WARNING` when it's actually consequential (i.e. the dashboard looks large by chart count but tiling would otherwise have been skipped); it's `DEBUG` for the common, benign case of a small dashboard with an unmeasured height. ### Not changed - The tiled-capture path itself (per-tile readiness, wait budgeting, etc.) — this PR only touches the earlier decision of *whether* to tile and the log level of one existing timeout. - Standard (non-tiled) capture behavior for dashboards that were already correctly identified as small. ### Testing Added to `tests/unit_tests/utils/webdriver_test.py`: - A large dashboard (by chart count) whose height can't be measured still attempts tiled capture instead of being silently downgraded to standard capture. - The chart-container timeout logs at `WARNING` with rendered/total progress and still raises (never logs via `logger.exception`). - Updated the existing "unknown height falls back" test to assert `DEBUG`, not `WARNING`, for the small-dashboard case where that's the expected/benign path. Full relevant test suite passing; `ruff check`, `ruff format`, and `mypy` clean on both changed files. ### 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 -- 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]
