deepujain commented on code in PR #44558:
URL: https://github.com/apache/superset/pull/44558#discussion_r4079758985
##########
superset/mcp_service/chart/tool/get_chart_preview.py:
##########
@@ -1087,6 +1094,100 @@ def generate(
return strategy.generate()
+async def _generate_png_preview(
+ chart_id: int, request: GetChartPreviewRequest
+) -> PNGPreview | ChartError:
+ """Render saved charts as the caller in an isolated browser and app
context."""
+ if (
+ not chart_id
+ or request.form_data_key
+ or request.extra_form_data
+ or guest_scope.is_guest_read()
+ ):
+ return ChartError(
+ error="PNG previews require a saved chart and a non-guest user, "
+ "without unsaved state or extra filters.",
+ error_type="UnsupportedFormat",
+ )
+ user_id = getattr(getattr(g, "user", None), "id", None)
+ if not isinstance(user_id, int):
+ return ChartError(error="Authentication required",
error_type="Forbidden")
+ width = 800 if request.width is None else request.width
+ height = 600 if request.height is None else request.height
+ if not (64 <= width <= 4096 and 64 <= height <= 4096):
+ return ChartError(
+ error="PNG dimensions must be between 64 and 4096 pixels.",
+ error_type="ValidationError",
+ )
+
+ def render() -> PNGPreview | ChartError:
+ from superset import is_feature_enabled
+ from superset.mcp_service.auth import _mcp_tool_call_context
+ from superset.utils.core import override_user
+ from superset.utils.screenshots import validate_screenshot_image
+ from superset.utils.urls import get_url_path
+ from superset.utils.webdriver import (
+ _PlaywrightBrowserManager,
+ WebDriverPlaywright,
+ )
+
+ # Resolve ORM objects in a fresh context; neither the request session
nor
+ # Flask's mutable g is shared with the rendering worker.
+ with _mcp_tool_call_context():
+ user = security_manager.find_user(id=user_id)
+ if user is None or not user.is_active:
+ return ChartError(
+ error="Authentication required", error_type="Forbidden"
+ )
+ with override_user(user):
+ chart = find_chart_by_identifier(chart_id)
+ if chart is None:
+ return ChartError(error="Chart not found",
error_type="NotFound")
+ security_manager.raise_for_access(chart=chart)
+ if is_feature_enabled(
+ "GRANULAR_EXPORT_CONTROLS"
+ ) and not security_manager.can_access("can_export_image",
"Superset"):
+ return ChartError(
+ error="Image export is not permitted",
error_type="Forbidden"
+ )
+ url = get_url_path(
+ "Superset.slice", slice_id=chart_id, standalone="true"
+ )
+ manager = _PlaywrightBrowserManager()
+ try:
+ driver = WebDriverPlaywright(
+ "",
+ (width, height),
+ require_complete_capture=True,
+ browser_manager=manager,
+ )
+ image = driver.get_screenshot(url, "chart-container",
user=user)
+ if (
+ image is None
+ or not image.startswith(b"\x89PNG\r\n\x1a\n")
+ or validate_screenshot_image(image)
+ ):
+ return ChartError(
+ error="Chart rendering failed",
error_type="RenderError"
+ )
+ with Image.open(BytesIO(image)) as rendered:
+ image_width, image_height = rendered.size
+ return PNGPreview(
+ data=base64.b64encode(image).decode("ascii"),
+ width=image_width,
+ height=image_height,
+ )
+ finally:
+ manager._cleanup()
+
+ try:
+ return await asyncio.to_thread(render)
Review Comment:
Acknowledged as a known limitation. `asyncio.to_thread` cannot be cancelled
mid-render, but occupation is bounded: the screenshot call has its own timeout
and `finally: manager._cleanup()` always releases the browser. Killing it early
would need process isolation, which is out of scope for this change.
--
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]
