rebenitez1802 commented on code in PR #43523:
URL: https://github.com/apache/superset/pull/43523#discussion_r3862463222


##########
superset/utils/screenshots.py:
##########
@@ -350,7 +398,9 @@ def compute_and_cache(  # pylint: disable=too-many-arguments
                     self.get_from_cache_key(cache_key) or 
ScreenshotCachePayload()
                 )
                 if not cache_payload.should_trigger_task(
-                    force=force, expected_scope=self.cache_scope
+                    force=force,
+                    expected_scope=self.cache_scope,
+                    check_updated_staleness=self.supports_updated_staleness,
                 ):

Review Comment:
   Fixed — `from_dict()` now preserves the stored `ERROR`/`COMPUTING` status; 
added round-trip tests. (The timestamp refreshes on those transitions, so it 
was masked as a fresh `UPDATED` rather than retried every request — same fix.)



##########
superset/dashboards/api.py:
##########
@@ -1996,7 +1996,9 @@ def build_response(status_code: int) -> WerkzeugResponse:
             )
 
         if cache_payload.should_trigger_task(
-            force, expected_scope=f"dashboard:{dashboard.id}"
+            force,
+            expected_scope=f"dashboard:{dashboard.id}",
+            check_updated_staleness=screenshot_obj.supports_updated_staleness,

Review Comment:
   Added `test_cache_dashboard_screenshot_recomputes_stale_updated` — fails if 
the `check_updated_staleness` arg is dropped.



##########
superset/dashboards/api.py:
##########
@@ -1996,7 +1996,9 @@ def build_response(status_code: int) -> WerkzeugResponse:
             )
 
         if cache_payload.should_trigger_task(
-            force, expected_scope=f"dashboard:{dashboard.id}"
+            force,
+            expected_scope=f"dashboard:{dashboard.id}",
+            check_updated_staleness=screenshot_obj.supports_updated_staleness,

Review Comment:
   Real but bounded — the `DistributedLock` still means a single render, same 
as the existing `PENDING`/`ERROR` triggers. Prefer an atomic pre-claim as a 
follow-up; can land it here if you'd rather.



-- 
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]

Reply via email to