sadpandajoe commented on code in PR #43523:
URL: https://github.com/apache/superset/pull/43523#discussion_r4152307639
##########
superset/charts/api.py:
##########
@@ -1274,16 +1288,23 @@ def screenshot(self, pk: int, digest: str) ->
WerkzeugResponse:
# serve its image under a different, merely-accessible `pk`.
if cache_payload.get_scope() != f"chart:{chart.id}":
return self.response_404()
- if cache_payload.status == StatusValues.UPDATED:
- try:
- image = cache_payload.get_image()
- except ScreenshotImageNotAvailableException:
- return self.response_404()
- return Response(
- FileWrapper(image),
- mimetype="image/png",
- direct_passthrough=True,
- )
+ # Serve whenever a valid image is present instead of gating on
+ # status == UPDATED. A failed forced refresh leaves the entry in an
+ # ERROR/COMPUTING backoff while still carrying the retained
last-good
+ # image; requiring UPDATED here would 404 that image for up to a
day.
+ # get_from_cache_key validates whatever image is present
regardless of
Review Comment:
When two charts share a cache key, a scope-mismatch refresh rebinds the
cached payload without clearing its image, so this reader can serve the
previous chart's bytes under the new chart's authorized URL while rendering or
after a failure. Could the worker discard retained bytes before changing a
mismatched scope?
##########
superset/dashboards/api.py:
##########
@@ -2046,6 +2046,10 @@ def should_enqueue(cache_payload:
ScreenshotCachePayload) -> bool:
force,
expected_scope=cache_scope,
force_retry_after_seconds=SCREENSHOT_API_FORCE_RETRY_SECONDS,
+ # Only DashboardScreenshot opts in: a stale-but-valid UPDATED
+ # generation older than THUMBNAIL_UPDATED_CACHE_TTL enqueues a
+ # refresh while the old generation stays servable.
+
check_updated_staleness=screenshot_obj.supports_updated_staleness,
Review Comment:
The image still disappears on the next force-less retry once this
generation's error TTL expires (`is_updated()` rejects it), and
`mark_cache_error_if_incomplete(...discard_image=True)` also drops it if broker
publication fails. Could valid, matching-scope bytes survive both retry
rotation and enqueue failure so `image_url` doesn't fall back to 404?
--
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]
