eschutho commented on PR #42624: URL: https://github.com/apache/superset/pull/42624#issuecomment-5148186424
Merged current master into this branch (covering Matt while he's out) — it was 5 relevant merges behind (#42273, #42153, #42120, #42118, plus a CroniterBadDateError fix). Conflicts were concentrated in `screenshot_utils.py`, `webdriver.py`, and the two test files, exactly where this PR's tiled/readiness work overlapped the recently merged PRs. Reconciliation decisions, called out explicitly since a couple override choices made in this branch before those merges landed: 1. **Tiled bounding: this PR's phase-based architecture kept, #42118's semantics folded in.** `_deadline_values`/`_timeout_seconds` remain the single mechanism (deadline-driven when a `report_execution_context` is present). Two #42118 behaviors were grafted onto the non-report fallback path: the fixed total ceiling when no Celery task budget exists (`TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS` — per-tile waits accumulate, so "no budget" must not mean uncapped, which this branch's previous fallback allowed), and the merged error-class family (`TiledScreenshotBudgetExceededError(ScreenshotTaskBudgetExceededError)`; this branch's duplicate `TimeoutError`-based definition removed — nothing catches it by base class). 2. **Unguarded thumbnail fallback NOT reintroduced.** This branch predated #42273's merge and deliberately preserved the raw-screenshot fallback for thumbnails on tiled failure. #42273 removed that fallback unconditionally after review (thumbnails handle the raise as a clean cache-ERROR; nothing blank is cached or served), so the merged code raises for all callers, keeping this PR's structured `terminal_reason=tiled_capture_failed` logging. If there's a strong case for the thumbnail exception it should be re-litigated against #42273's rationale rather than slipped back in via merge. 3. **#42153's changes composed cleanly**: the unknown-height tiling routing and chart-container progress counting are preserved alongside this PR's per-element deadline-bounded `wait_for` and expected-chart-count hint. 4. **Animation wait**: report-mode reserve logic kept; non-report mode now caps at remaining budget and skips (rather than raises) on exhaustion, with the per-tile timing DEBUG line from #42118 retained. 5. **Budget-exhaustion behavior in the merged design**: the pre-capture `screenshot_capture` phase check (and the deadline-bounded `page.screenshot` call) governs — #42118's tests were updated accordingly (exhaustion now aborts before capturing the tile whose readiness wait consumed the budget; the bottom handler logs the structured `report_capture_terminal` WARNING). Note this is coherent where the bare "re-check before capture" suggestion declined on #42118 was not, because here the capture call itself is time-bounded. Verification: `tests/unit_tests/utils/` + `tests/unit_tests/commands/report/` — **1,017 passed**; ruff check/format clean. Follow-ups #42657 (tracing) and #42661 (tiled clock anchor) remain open and will need small mechanical rebases against whichever of this PR/them lands first. -- 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]
