msyavuz commented on code in PR #44465:
URL: https://github.com/apache/superset/pull/44465#discussion_r4059857465
##########
superset/utils/webdriver.py:
##########
@@ -333,13 +336,16 @@ def _get_validated_screenshot(
else REPORT_HAS_RENDERED_CHART_HOLDERS_JS
)
)
- except PlaywrightError:
- has_rendered_content = False
+ except PlaywrightError as ex:
+
report_execution_context.reject_capture("content_state_unknown")
logger.warning(
"report_capture_content_state_failed capture=standard%s",
context_suffix,
exc_info=True,
)
+ raise ScreenshotBlankCaptureError(
Review Comment:
`for_delivery=False` doesn't bypass this, so the CSV/XLSX/TEXT query-context
bootstrap still passes the execution context and a flaky content-state
`evaluate` now fails the tabular report (previously
`has_rendered_content=False` and it continued). That seems to contradict
"retaining the existing browser readiness/capture contract" for bootstrap
captures.
##########
superset/utils/screenshot_utils.py:
##########
@@ -145,6 +148,31 @@ class ScreenshotBlankCaptureError(RuntimeError):
"""Raised when Chromium repeatedly returns a perceptually blank capture."""
+def validate_report_screenshot(
+ screenshot: bytes,
+ context: ReportExecutionContext,
+) -> None:
+ """Validate exact image bytes independently of browser or cache
provenance."""
+ if context.capture_was_rejected:
+ raise ScreenshotBlankCaptureError("Capture was already rejected")
+ if context.artifact_was_validated(screenshot):
+ return
+ try:
+ with Image.open(io.BytesIO(screenshot)) as image:
+ image.verify()
+ blankness = get_screenshot_blankness_metrics(screenshot)
+ except (SoftTimeLimitExceeded, ReportExecutionBudgetExceededError):
+ context.reject_capture("validation_interrupted")
+ raise
+ except Exception as ex:
+ context.reject_capture("invalid_image")
+ raise ScreenshotBlankCaptureError("Unable to validate screenshot
bytes") from ex
+ if blankness.is_blank:
Review Comment:
Both capture paths accept a blank image when no holder has rendered content
(`is_blank = has_rendered_content and ...`), but this gate then rejects the
same bytes with no retry. Is failing an empty/error-only chart or dashboard on
the first attempt intended?
--
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]
