Skip to content

Commit

Permalink
feat: Implement using Playwright for taking screenshots in reports (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
kgabryje authored Oct 4, 2023
1 parent 5301339 commit ff95d0f
Show file tree
Hide file tree
Showing 11 changed files with 340 additions and 78 deletions.
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
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
# 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
68 changes: 58 additions & 10 deletions superset/utils/machine_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
from __future__ import annotations

import logging
from typing import Callable, TYPE_CHECKING
from typing import Any, Callable, TYPE_CHECKING
from urllib.parse import urlparse

from flask import current_app, Flask, request, Response, session
from flask_login import login_user
Expand All @@ -33,14 +34,24 @@
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
# This is here in order to allow for the authenticate_webdriver
# or authenticate_browser_context (if PLAYWRIGHT_REPORTS_AND_THUMBNAILS is
# enabled) 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

def authenticate_webdriver(
self,
Expand All @@ -58,17 +69,54 @@ 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)

url = urlparse(current_app.config["WEBDRIVER_BASEURL"])

# 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": url.netloc,
"path": "/",
"sameSite": "Lax",
"httpOnly": True,
}
for cookie_name, cookie_val in cookies.items()
]
)
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 +150,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"])

This comment has been minimized.

Copy link
@eschutho

eschutho Oct 4, 2023

Member

@kgabryje it looks like this may break existing configs because it is changing the amount of arguments in the init function definition of the class.


@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

0 comments on commit ff95d0f

Please sign in to comment.