rebenitez1802 commented on PR #43523: URL: https://github.com/apache/superset/pull/43523#issuecomment-5767351234
Thanks @rusackas — both you flagged are already fixed in code (the threads just weren't marked resolved): the ERROR/COMPUTING→UPDATED clobber (`screenshots.py:142` restores the persisted status instead of re-inferring UPDATED from a retained image) and the `TTL=0` opt-out (docs-only fix — `cache.mdx:344-347` now says it disables the new freshness check and falls back to the backend's own retention, "indefinite" only on no-eviction backends like S3). You were right to be suspicious though — there *was* one real open bug, just not those: on the chart `cache_screenshot` path the last commit pre-wrote `COMPUTING` before enqueuing, so a force-less request (`force` defaults to `None`) made the worker re-read a fresh COMPUTING entry, skip, and the screenshot never rendered (it regressed even first render). Just pushed a fix — charts now mirror the dashboard endpoint (no pre-write), and I replaced the `force=True`/mocked-`.delay` test that masked it with two force-less regression tests. I'll resolve the addressed threads. Only deferred item: the non-atomic check-then-`.delay()` can enqueue redundant tasks under a burst, but it's bounded to a single render by the worker `DistributedLock` (dupes no-op) and is a pre-existing pattern on these endpoints — I'd do the atomic claim as a follow-up across all of them unless you'd rather here. -- 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]
