Revert "feat: Implement using Playwright for taking screenshots in re…

…ports (apache#25247)"

This reverts commit ff95d0f.

RESOURCES/FEATURE_FLAGS.md

@@ -52,7 +52,6 @@ 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

docker/docker-bootstrap.sh

@@ -35,14 +35,6 @@ 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..."

requirements/testing.in

@@ -16,7 +16,7 @@
 #
 -r development.in
 -r integration.in
--e file:.[bigquery,hive,presto,prophet,trino,gsheets,playwright]
+-e file:.[bigquery,hive,presto,prophet,trino,gsheets]
 docker
 flask-testing
 freezegun

requirements/testing.txt

@@ -1,4 +1,4 @@
-# SHA1:95300275481abb1413eb98a5c79fb7cf96814cdd
+# SHA1:78d0270a4f583095e0587aa21f57fc2ff7fe8b84
 #
 # This file is autogenerated by pip-compile-multi
 # To update, run:
@@ -104,8 +104,6 @@ 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

setup.py

@@ -183,7 +183,6 @@ 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"],

superset-frontend/src/components/ErrorMessage/ErrorAlert.tsx

@@ -183,7 +183,6 @@ export default function ErrorAlert({
           level={level}
           show={isModalOpen}
           onHide={() => setIsModalOpen(false)}
-          destroyOnClose
           title={
             <div className="header">
               {level === 'error' ? (

superset/config.py

@@ -500,10 +500,6 @@ 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,
 }
 
 # ------------------------------
@@ -1351,11 +1347,9 @@ 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 (when using Selenium) or browser context (when using Playwright - see
-# PLAYWRIGHT_REPORTS_AND_THUMBNAILS feature flag)
+# An optional override to the default auth hook used to provide auth to the
+# offline webdriver
 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"}

superset/utils/machine_auth.py

@@ -18,8 +18,7 @@
 from __future__ import annotations
 
 import logging
-from typing import Any, Callable, TYPE_CHECKING
-from urllib.parse import urlparse
+from typing import Callable, TYPE_CHECKING
 
 from flask import current_app, Flask, request, Response, session
 from flask_login import login_user
@@ -34,24 +33,14 @@
 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],
-        auth_context_func_override: Callable[[BrowserContext, User], BrowserContext],
+        self, auth_webdriver_func_override: Callable[[WebDriver, User], WebDriver]
     ):
-        # 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
+        # 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
 
     def authenticate_webdriver(
         self,
@@ -69,54 +58,17 @@ 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 = {}
-        return cookies
+
+        for cookie_name, cookie_val in cookies.items():
+            driver.add_cookie({"name": cookie_name, "value": cookie_val})
+
+        return driver
 
     @staticmethod
     def get_auth_cookies(user: User) -> dict[str, str]:
@@ -150,7 +102,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["BROWSER_CONTEXT_AUTH_FUNC"])
+        )(app.config["WEBDRIVER_AUTH_FUNC"])
 
     @property
     def instance(self) -> MachineAuthProvider:

superset/utils/screenshots.py

@@ -22,15 +22,12 @@
 
 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,
-    WebDriver,
-    WebDriverPlaywright,
-    WebDriverSelenium,
+    WebDriverProxy,
     WindowSize,
 )
 
@@ -64,11 +61,9 @@ def __init__(self, url: str, digest: str):
         self.url = url
         self.screenshot: bytes | None = None
 
-    def driver(self, window_size: WindowSize | None = None) -> WebDriver:
+    def driver(self, window_size: WindowSize | None = None) -> WebDriverProxy:
         window_size = window_size or self.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)
+        return WebDriverProxy(self.driver_type, window_size)
 
     def cache_key(
         self,