aminghadersohi commented on code in PR #44114:
URL: https://github.com/apache/superset/pull/44114#discussion_r3980370512
##########
superset/utils/screenshot_utils.py:
##########
@@ -516,6 +546,35 @@ def _unready_chart_holders_js_body(*, viewport_only: bool)
-> str:
);
}}
"""
+STABLE_CHART_CONTAINER_READY_JS =
_stable_readiness_js(CHART_CONTAINER_READY_JS)
+
+
+def wait_for_stable_readiness(
+ page: Page,
+ readiness_predicate: str,
+ timeout_seconds: float,
+) -> bool:
+ """Wait for capture readiness when the available budget can satisfy the
dwell.
+
+ Returns ``False`` when less than one stability window remains. The caller
may
+ proceed because the preceding readiness gate has already completed;
starting
+ an impossible dwell would only convert a usable capture budget into a
timeout.
+ """
+
+ minimum_timeout_seconds = REPORT_CAPTURE_READINESS_STABILITY_MS / 1000
+ if timeout_seconds <= minimum_timeout_seconds:
Review Comment:
The first poll only records `since`, so success needs >500 ms — an
always-ready page first returns true at 512 ms under rAF polling. A budget in
(500, 512] ms times out and raises `report_capture_readiness_changed`, failing
the report while capture reserve is still intact.
```suggestion
# The first poll only records the dwell start, so success needs the full
# stability window plus at least one polling interval on top of it.
minimum_timeout_seconds = (REPORT_CAPTURE_READINESS_STABILITY_MS + 250)
/ 1000
if timeout_seconds <= minimum_timeout_seconds:
```
##########
docs/admin_docs/configuration/alerts-reports.mdx:
##########
@@ -486,6 +486,13 @@ applies to alerts that attach reports. Check worker logs
for
`ScreenshotBlankCaptureError` and `report_capture_validation` when
investigating
these failures.
+Immediately before each scheduled report screenshot, chart readiness must
remain
+stable for 500 milliseconds. This final check runs after scrolling, content
Review Comment:
`wait_for_stable_readiness` skips the dwell entirely when less than one
stability window of budget remains (logged `skipped=True`), so this is not
unconditional — and budget-starved reports are exactly the ones the race
affects.
```suggestion
Immediately before each scheduled report screenshot, chart readiness must
remain
stable for 500 milliseconds, unless less than one stability window of the
execution budget is left, in which case the check is skipped and logged with
`skipped=True`. This final check runs after scrolling, content
```
--
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]
