Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Implement using Playwright for taking screenshots in reports #25247

Merged
merged 16 commits into from
Oct 4, 2023
Merged
1 change: 1 addition & 0 deletions RESOURCES/FEATURE_FLAGS.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ These features are **finished** but currently being tested. They are usable, but
- GENERIC_CHART_AXES
- GLOBAL_ASYNC_QUERIES [(docs)](https://github.com/apache/superset/blob/master/CONTRIBUTING.md#async-chart-queries)
- HORIZONTAL_FILTER_BAR
- PLAYWRIGHT_REPORTS_AND_THUMBNAILS
- RLS_IN_SQLLAB
- SSH_TUNNELING [(docs)](https://superset.apache.org/docs/installation/setup-ssh-tunneling)
- USE_ANALAGOUS_COLORS
Expand Down
8 changes: 8 additions & 0 deletions docker/docker-bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ else
echo "Skipping local overrides"
fi

#
# playwright is an optional package - run only if it is installed
#
if command -v playwright > /dev/null 2>&1; then
playwright install-deps
playwright install chromium
villebro marked this conversation as resolved.
Show resolved Hide resolved
fi

case "${1}" in
worker)
echo "Starting Celery worker..."
Expand Down
2 changes: 1 addition & 1 deletion requirements/testing.in
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
#
-r development.in
-r integration.in
-e file:.[bigquery,hive,presto,prophet,trino,gsheets]
-e file:.[bigquery,hive,presto,prophet,trino,gsheets,playwright]
docker
flask-testing
freezegun
Expand Down
4 changes: 3 additions & 1 deletion requirements/testing.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SHA1:78d0270a4f583095e0587aa21f57fc2ff7fe8b84
# SHA1:95300275481abb1413eb98a5c79fb7cf96814cdd
#
# This file is autogenerated by pip-compile-multi
# To update, run:
Expand Down Expand Up @@ -104,6 +104,8 @@ parameterized==0.9.0
# via -r requirements/testing.in
pathable==0.4.3
# via jsonschema-spec
playwright==1.37.0
# via apache-superset
prophet==1.1.1
# via apache-superset
proto-plus==1.22.2
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ def get_git_sha() -> str:
],
"oracle": ["cx-Oracle>8.0.0, <8.1"],
"pinot": ["pinotdb>=0.3.3, <0.4"],
"playwright": ["playwright>=1.37.0, <2"],
"postgres": ["psycopg2-binary==2.9.6"],
"presto": ["pyhive[presto]>=0.6.5"],
"trino": ["trino>=0.324.0"],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@ export default function ErrorAlert({
level={level}
show={isModalOpen}
onHide={() => setIsModalOpen(false)}
destroyOnClose
title={
<div className="header">
{level === 'error' ? (
Expand Down
10 changes: 8 additions & 2 deletions superset/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,10 @@ class D3Format(TypedDict, total=False):
# returned from each database in the ``SUPERSET_META_DB_LIMIT`` configuration value
# in this file.
"ENABLE_SUPERSET_META_DB": False,
# Set to True to replace Selenium with Playwright to execute reports and thumbnails.
# Unlike Selenium, Playwright reports support deck.gl visualizations
eschutho marked this conversation as resolved.
Show resolved Hide resolved
# Enabling this feature flag requires installing "playwright" pip package
"PLAYWRIGHT_REPORTS_AND_THUMBNAILS": False,
}

# ------------------------------
Expand Down Expand Up @@ -1347,9 +1351,11 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument
"pixel_density": 1,
}

# An optional override to the default auth hook used to provide auth to the
# offline webdriver
# An optional override to the default auth hook used to provide auth to the offline
# webdriver (when using Selenium) or browser context (when using Playwright - see
# PLAYWRIGHT_REPORTS_AND_THUMBNAILS feature flag)
WEBDRIVER_AUTH_FUNC = None
BROWSER_CONTEXT_AUTH_FUNC = None

# Any config options to be passed as-is to the webdriver
WEBDRIVER_CONFIGURATION: dict[Any, Any] = {"service_log_path": "/dev/null"}
Expand Down
59 changes: 51 additions & 8 deletions superset/utils/machine_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
from __future__ import annotations

import logging
from typing import Callable, TYPE_CHECKING
from typing import Any, Callable, TYPE_CHECKING

from flask import current_app, Flask, request, Response, session
from flask_login import login_user
Expand All @@ -33,14 +33,22 @@
if TYPE_CHECKING:
from flask_appbuilder.security.sqla.models import User

try:
from playwright.sync_api import BrowserContext
except ModuleNotFoundError:
BrowserContext = Any


class MachineAuthProvider:
def __init__(
self, auth_webdriver_func_override: Callable[[WebDriver, User], WebDriver]
self,
auth_webdriver_func_override: Callable[[WebDriver, User], WebDriver],
auth_context_func_override: Callable[[BrowserContext, User], BrowserContext],
):
# This is here in order to allow for the authenticate_webdriver func to be
# overridden via config, as opposed to the entire provider implementation
self._auth_webdriver_func_override = auth_webdriver_func_override
self._auth_context_func_override = auth_context_func_override
villebro marked this conversation as resolved.
Show resolved Hide resolved

def authenticate_webdriver(
self,
Expand All @@ -58,17 +66,52 @@ def authenticate_webdriver(
# Setting cookies requires doing a request first
driver.get(headless_url("/login/"))

cookies = self.get_cookies(user)

for cookie_name, cookie_val in cookies.items():
driver.add_cookie({"name": cookie_name, "value": cookie_val})

return driver

def authenticate_browser_context(
self,
browser_context: BrowserContext,
user: User,
) -> BrowserContext:
# Short-circuit this method if we have an override configured
if self._auth_context_func_override: # type: ignore
return self._auth_context_func_override(browser_context, user)

# Setting cookies requires doing a request first
page = browser_context.new_page()
page.goto(headless_url("/login/"))

cookies = self.get_cookies(user)

browser_context.clear_cookies()
browser_context.add_cookies(
[
{
"name": cookie_name,
"value": cookie_val,
"domain": "0.0.0.0",
"path": "/",
"sameSite": "Lax",
"httpOnly": True,
}
for cookie_name, cookie_val in cookies.items()
]
)
villebro marked this conversation as resolved.
Show resolved Hide resolved
return browser_context

def get_cookies(self, user: User | None) -> dict[str, str]:
if user:
cookies = self.get_auth_cookies(user)
elif request.cookies:
cookies = request.cookies
else:
cookies = {}

for cookie_name, cookie_val in cookies.items():
driver.add_cookie({"name": cookie_name, "value": cookie_val})

return driver
return cookies

@staticmethod
def get_auth_cookies(user: User) -> dict[str, str]:
Expand Down Expand Up @@ -102,7 +145,7 @@ def __init__(self) -> None:
def init_app(self, app: Flask) -> None:
self._auth_provider = load_class_from_name(
app.config["MACHINE_AUTH_PROVIDER_CLASS"]
)(app.config["WEBDRIVER_AUTH_FUNC"])
)(app.config["WEBDRIVER_AUTH_FUNC"], app.config["BROWSER_CONTEXT_AUTH_FUNC"])

@property
def instance(self) -> MachineAuthProvider:
Expand Down
11 changes: 8 additions & 3 deletions superset/utils/screenshots.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,15 @@

from flask import current_app

from superset import feature_flag_manager
from superset.utils.hashing import md5_sha_from_dict
from superset.utils.urls import modify_url_query
from superset.utils.webdriver import (
ChartStandaloneMode,
DashboardStandaloneMode,
WebDriverProxy,
WebDriver,
WebDriverPlaywright,
WebDriverSelenium,
WindowSize,
)

Expand Down Expand Up @@ -61,9 +64,11 @@ def __init__(self, url: str, digest: str):
self.url = url
self.screenshot: bytes | None = None

def driver(self, window_size: WindowSize | None = None) -> WebDriverProxy:
def driver(self, window_size: WindowSize | None = None) -> WebDriver:
window_size = window_size or self.window_size
return WebDriverProxy(self.driver_type, window_size)
if feature_flag_manager.is_feature_enabled("PLAYWRIGHT_REPORTS_AND_THUMBNAILS"):
return WebDriverPlaywright(self.driver_type, window_size)
return WebDriverSelenium(self.driver_type, window_size)

def cache_key(
self,
Expand Down
Loading
Loading