rebenitez1802 opened a new pull request, #43523: URL: https://github.com/apache/superset/pull/43523
### SUMMARY `ScreenshotCachePayload.should_trigger_task()` (in `superset/utils/screenshots.py`) decides whether a screenshot is recomputed or served from cache. It has time-based staleness escape hatches for `ERROR` (`is_error_cache_ttl_expired`) and `COMPUTING` (`is_computing_stale`) entries, but the `UPDATED` branch only re-triggers when the image is missing or the scope mismatches — there is **no freshness/TTL check** on a successfully-rendered entry. As a result, once an image is cached as `UPDATED` — including a structurally valid but visually blank PNG — a force-less caller serves it indefinitely. On cache backends without TTL eviction (e.g. S3), "indefinitely" is literal. Superset's own "Download as Image/PDF" UI flow dodges this by always sending `force: true`, but a caller hitting `POST /api/v1/dashboard/<pk>/cache_dashboard_screenshot/` directly (without `force`) has no such protection: a single bad capture can get stuck in cache and be served forever. This PR adds a time-based freshness check for `UPDATED` entries, mirroring the existing `ERROR`/`COMPUTING` staleness handling: - New config `THUMBNAIL_UPDATED_CACHE_TTL` (default 7 days, mirroring `THUMBNAIL_CACHE_CONFIG`'s `CACHE_DEFAULT_TIMEOUT`). Set to `0`/`None` to disable, which restores the previous serve-forever behavior (instant, no-code rollback). No cache-format change and **no DB migration** — it reuses the timestamp already stored on each cache payload. - New `ScreenshotCachePayload.is_updated_stale()` helper and a corresponding clause in `should_trigger_task`. An unparseable/uncomparable (legacy) timestamp is logged and treated as stale so it self-heals. **Design decision — scoped to the dashboard on-demand endpoint.** The recompute is opt-in per screenshot type via a new `BaseScreenshot.supports_updated_staleness` flag (threaded as a `check_updated_staleness` argument into `should_trigger_task`), enabled only where the serving path degrades gracefully: - **Dashboard `cache_dashboard_screenshot`** does not pre-wipe the cache entry and its serving GET returns whenever an image is present, so recomputing a stale-but-valid capture degrades gracefully — the old image stays servable while the refresh is in flight. **Opted in.** - **Chart `cache_screenshot`** pre-wipes the entry to `PENDING` before recompute and its serving GET returns an image only when `status == UPDATED`, so a stale-driven recompute could surface a transient 404 (or a longer one if the recompute fails). **Left unchanged.** - The high-traffic **card-thumbnail list** endpoints call `should_trigger_task()` without a scope and are unaffected. Note: `superset compute-thumbnails` run without `--force` will also refresh stale-but-valid **dashboard** thumbnails (charts remain excluded). ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A — backend-only change; no UI. ### TESTING INSTRUCTIONS Unit tests: ``` pytest tests/unit_tests/utils/test_screenshot_cache_fix.py tests/unit_tests/utils/screenshot_test.py ``` Added coverage: `is_updated_stale` boundary / disabled (`0`/`None`) / malformed- and tz-aware-timestamp cases; scoped `should_trigger_task` staleness gating; an end-to-end `DashboardScreenshot` test that a stale-but-valid entry **is** recomputed; and a `ChartScreenshot` test that a stale-but-valid entry is **not** recomputed (chart behavior unchanged). Manual: set `THUMBNAIL_UPDATED_CACHE_TTL` to a small value, then `POST /api/v1/dashboard/<pk>/cache_dashboard_screenshot/` (no `force`). Once the cached entry is older than the TTL the endpoint returns `202` and recomputes instead of serving the stale image; set the TTL to `0`/`None` to confirm the prior serve-forever behavior is restored. ### 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]
