mirror of
https://github.com/apache/superset.git
synced 2026-06-03 14:49:23 +00:00
fix(utils): ensure webdriver timeout compatibility with urllib3 2.x (#34440)
This commit is contained in:
Elizabeth Thompson
committed by
GitHub
GitHub
parent
bef1f4d045
commit
385471c34d
@@ -21,7 +21,7 @@ import logging
|
||||
from abc import ABC, abstractmethod
|
||||
from enum import Enum
|
||||
from time import sleep
|
||||
from typing import TYPE_CHECKING
|
||||
from typing import Any, TYPE_CHECKING
|
||||
|
||||
from flask import current_app as app
|
||||
from packaging import version
|
||||
@@ -302,47 +302,99 @@ class WebDriverPlaywright(WebDriverProxy):
|
||||
|
||||
|
||||
class WebDriverSelenium(WebDriverProxy):
|
||||
def _create_firefox_driver(
|
||||
self, pixel_density: float
|
||||
) -> tuple[type[WebDriver], type[Service], dict[str, Any]]:
|
||||
"""Create Firefox driver configuration."""
|
||||
options = firefox.options.Options()
|
||||
profile = FirefoxProfile()
|
||||
profile.set_preference("layout.css.devPixelsPerPx", str(pixel_density))
|
||||
options.profile = profile
|
||||
return (
|
||||
firefox.webdriver.WebDriver,
|
||||
firefox.service.Service,
|
||||
{"options": options},
|
||||
)
|
||||
|
||||
def _create_chrome_driver(
|
||||
self, pixel_density: float
|
||||
) -> tuple[type[WebDriver], type[Service], dict[str, Any]]:
|
||||
"""Create Chrome driver configuration."""
|
||||
options = chrome.options.Options()
|
||||
options.add_argument(f"--force-device-scale-factor={pixel_density}")
|
||||
options.add_argument(f"--window-size={self._window[0]},{self._window[1]}")
|
||||
return (
|
||||
chrome.webdriver.WebDriver,
|
||||
chrome.service.Service,
|
||||
{"options": options},
|
||||
)
|
||||
|
||||
def _normalize_timeout_values(self, config: dict[str, Any]) -> dict[str, Any]:
|
||||
"""Convert timeout values to float for urllib3 2.x compatibility."""
|
||||
timeout_keys = [
|
||||
"timeout",
|
||||
"connect_timeout",
|
||||
"socket_timeout",
|
||||
"read_timeout",
|
||||
"page_load_timeout",
|
||||
"implicit_wait",
|
||||
"command_executor_timeout",
|
||||
"connection_timeout",
|
||||
]
|
||||
|
||||
for key, value in config.items():
|
||||
if any(timeout_key in key.lower() for timeout_key in timeout_keys):
|
||||
if value is None or value == "None" or value == "null":
|
||||
config[key] = None
|
||||
else:
|
||||
try:
|
||||
config[key] = float(value)
|
||||
except (ValueError, TypeError):
|
||||
config[key] = None
|
||||
logger.warning(
|
||||
f"Invalid timeout value for {key}: {value}, setting to None"
|
||||
)
|
||||
return config
|
||||
|
||||
def create(self) -> WebDriver:
|
||||
pixel_density = app.config["WEBDRIVER_WINDOW"].get("pixel_density", 1)
|
||||
|
||||
# Get driver class and initial kwargs based on driver type
|
||||
if self._driver_type == "firefox":
|
||||
driver_class: type[WebDriver] = firefox.webdriver.WebDriver
|
||||
service_class: type[Service] = firefox.service.Service
|
||||
options = firefox.options.Options()
|
||||
profile = FirefoxProfile()
|
||||
profile.set_preference("layout.css.devPixelsPerPx", str(pixel_density))
|
||||
options.profile = profile
|
||||
kwargs = {"options": options}
|
||||
driver_class, service_class, kwargs = self._create_firefox_driver(
|
||||
pixel_density
|
||||
)
|
||||
elif self._driver_type == "chrome":
|
||||
driver_class = chrome.webdriver.WebDriver
|
||||
service_class = chrome.service.Service
|
||||
options = chrome.options.Options()
|
||||
options.add_argument(f"--force-device-scale-factor={pixel_density}")
|
||||
options.add_argument(f"--window-size={self._window[0]},{self._window[1]}")
|
||||
kwargs = {"options": options}
|
||||
driver_class, service_class, kwargs = self._create_chrome_driver(
|
||||
pixel_density
|
||||
)
|
||||
else:
|
||||
raise Exception( # pylint: disable=broad-exception-raised
|
||||
f"Webdriver name ({self._driver_type}) not supported"
|
||||
)
|
||||
|
||||
# Prepare args for the webdriver init
|
||||
# Add additional arguments from config
|
||||
options = kwargs["options"]
|
||||
for arg in list(app.config["WEBDRIVER_OPTION_ARGS"]):
|
||||
options.add_argument(arg)
|
||||
|
||||
# Add additional configured webdriver options
|
||||
webdriver_conf = dict(app.config["WEBDRIVER_CONFIGURATION"])
|
||||
# Fix timeout values for urllib3 2.x compatibility
|
||||
webdriver_config = app.config["WEBDRIVER_CONFIGURATION"].copy()
|
||||
webdriver_config = self._normalize_timeout_values(webdriver_config)
|
||||
kwargs.update(webdriver_config)
|
||||
|
||||
# Set the binary location if provided
|
||||
# We need to pop it from the dict due to selenium_version < 4.10.0
|
||||
options.binary_location = webdriver_conf.pop("binary_location", "")
|
||||
options.binary_location = webdriver_config.pop("binary_location", "")
|
||||
|
||||
if version.parse(selenium_version) < version.parse("4.10.0"):
|
||||
kwargs |= webdriver_conf
|
||||
kwargs |= webdriver_config
|
||||
else:
|
||||
driver_opts = dict(
|
||||
webdriver_conf.get("options", {"capabilities": {}, "preferences": {}})
|
||||
webdriver_config.get("options", {"capabilities": {}, "preferences": {}})
|
||||
)
|
||||
driver_srv = dict(
|
||||
webdriver_conf.get(
|
||||
webdriver_config.get(
|
||||
"service",
|
||||
{
|
||||
"log_output": "/dev/null",
|
||||
|
||||
263
tests/unit_tests/utils/webdriver_test.py
Normal file
263
tests/unit_tests/utils/webdriver_test.py
Normal file
@@ -0,0 +1,263 @@
|
||||
# Licensed to the Apache Software Foundation (ASF) under one
|
||||
# or more contributor license agreements. See the NOTICE file
|
||||
# distributed with this work for additional information
|
||||
# regarding copyright ownership. The ASF licenses this file
|
||||
# to you under the Apache License, Version 2.0 (the
|
||||
# "License"); you may not use this file except in compliance
|
||||
# with the License. You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing,
|
||||
# software distributed under the License is distributed on an
|
||||
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
# KIND, either express or implied. See the License for the
|
||||
# specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from superset.utils.webdriver import WebDriverSelenium
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def mock_app():
|
||||
"""Mock Flask app with webdriver configuration."""
|
||||
app = MagicMock()
|
||||
app.config = {
|
||||
"WEBDRIVER_TYPE": "chrome",
|
||||
"WEBDRIVER_OPTION_ARGS": [],
|
||||
"WEBDRIVER_CONFIGURATION": {},
|
||||
"SCREENSHOT_LOCATE_WAIT": 10,
|
||||
"SCREENSHOT_LOAD_WAIT": 10,
|
||||
}
|
||||
return app
|
||||
|
||||
|
||||
class TestWebDriverSelenium:
|
||||
"""Test WebDriverSelenium timeout handling for urllib3 2.x compatibility."""
|
||||
|
||||
@patch("superset.utils.webdriver.app")
|
||||
@patch("superset.utils.webdriver.firefox")
|
||||
@patch("superset.utils.webdriver.chrome")
|
||||
def test_timeout_conversion_to_float(
|
||||
self, mock_chrome, mock_firefox, mock_app_patch, mock_app
|
||||
):
|
||||
"""Test that timeout values are properly converted to float."""
|
||||
# Set up app mock to be used throughout
|
||||
mock_app_patch.config = {
|
||||
"WEBDRIVER_TYPE": "chrome",
|
||||
"WEBDRIVER_OPTION_ARGS": [],
|
||||
"SCREENSHOT_LOCATE_WAIT": 10,
|
||||
"SCREENSHOT_LOAD_WAIT": 10,
|
||||
"WEBDRIVER_WINDOW": {},
|
||||
"WEBDRIVER_CONFIGURATION": {
|
||||
"timeout": "30",
|
||||
"connect_timeout": "10.5",
|
||||
"socket_timeout": 20,
|
||||
"read_timeout": "15.0",
|
||||
"command_executor_timeout": "25",
|
||||
},
|
||||
}
|
||||
|
||||
mock_driver_class = MagicMock()
|
||||
mock_chrome.webdriver.WebDriver = mock_driver_class
|
||||
mock_chrome.service.Service = MagicMock()
|
||||
mock_options = MagicMock()
|
||||
mock_options.add_argument = MagicMock()
|
||||
mock_chrome.options.Options = MagicMock(return_value=mock_options)
|
||||
|
||||
driver = WebDriverSelenium(driver_type="chrome")
|
||||
driver.create()
|
||||
|
||||
# Check that the driver was called with float timeout values
|
||||
mock_driver_class.assert_called_once()
|
||||
call_kwargs = mock_driver_class.call_args.kwargs
|
||||
assert call_kwargs["timeout"] == 30.0
|
||||
assert call_kwargs["connect_timeout"] == 10.5
|
||||
assert call_kwargs["socket_timeout"] == 20.0
|
||||
assert call_kwargs["read_timeout"] == 15.0
|
||||
assert call_kwargs["command_executor_timeout"] == 25.0
|
||||
|
||||
@patch("superset.utils.webdriver.app")
|
||||
@patch("superset.utils.webdriver.chrome")
|
||||
def test_timeout_none_handling(self, mock_chrome, mock_app_patch, mock_app):
|
||||
"""Test that None, 'None', and 'null' timeout values are set to None."""
|
||||
mock_app_patch.config = {
|
||||
"WEBDRIVER_TYPE": "chrome",
|
||||
"WEBDRIVER_OPTION_ARGS": [],
|
||||
"SCREENSHOT_LOCATE_WAIT": 10,
|
||||
"SCREENSHOT_LOAD_WAIT": 10,
|
||||
"WEBDRIVER_WINDOW": {},
|
||||
"WEBDRIVER_CONFIGURATION": {
|
||||
"timeout": None,
|
||||
"connect_timeout": "None",
|
||||
"socket_timeout": "null",
|
||||
},
|
||||
}
|
||||
|
||||
mock_driver_class = MagicMock()
|
||||
mock_chrome.webdriver.WebDriver = mock_driver_class
|
||||
mock_chrome.service.Service = MagicMock()
|
||||
mock_options = MagicMock()
|
||||
mock_options.add_argument = MagicMock()
|
||||
mock_chrome.options.Options = MagicMock(return_value=mock_options)
|
||||
|
||||
driver = WebDriverSelenium(driver_type="chrome")
|
||||
driver.create()
|
||||
|
||||
# Check that None values are preserved
|
||||
mock_driver_class.assert_called_once()
|
||||
call_kwargs = mock_driver_class.call_args.kwargs
|
||||
assert call_kwargs["timeout"] is None
|
||||
assert call_kwargs["connect_timeout"] is None
|
||||
assert call_kwargs["socket_timeout"] is None
|
||||
|
||||
@patch("superset.utils.webdriver.app")
|
||||
@patch("superset.utils.webdriver.chrome")
|
||||
@patch("superset.utils.webdriver.logger")
|
||||
def test_invalid_timeout_warning(
|
||||
self, mock_logger, mock_chrome, mock_app_patch, mock_app
|
||||
):
|
||||
"""Test that invalid timeout values log warnings and are set to None."""
|
||||
mock_app_patch.config = {
|
||||
"WEBDRIVER_TYPE": "chrome",
|
||||
"WEBDRIVER_OPTION_ARGS": [],
|
||||
"SCREENSHOT_LOCATE_WAIT": 10,
|
||||
"SCREENSHOT_LOAD_WAIT": 10,
|
||||
"WEBDRIVER_WINDOW": {},
|
||||
"WEBDRIVER_CONFIGURATION": {
|
||||
"timeout": "invalid",
|
||||
"connect_timeout": "not_a_number",
|
||||
"Page_Load_Timeout": "abc123", # Test case-insensitive matching
|
||||
},
|
||||
}
|
||||
|
||||
mock_driver_class = MagicMock()
|
||||
mock_chrome.webdriver.WebDriver = mock_driver_class
|
||||
mock_chrome.service.Service = MagicMock()
|
||||
mock_options = MagicMock()
|
||||
mock_options.add_argument = MagicMock()
|
||||
mock_chrome.options.Options = MagicMock(return_value=mock_options)
|
||||
|
||||
driver = WebDriverSelenium(driver_type="chrome")
|
||||
driver.create()
|
||||
|
||||
# Check that invalid values are set to None
|
||||
mock_driver_class.assert_called_once()
|
||||
call_kwargs = mock_driver_class.call_args.kwargs
|
||||
assert call_kwargs["timeout"] is None
|
||||
assert call_kwargs["connect_timeout"] is None
|
||||
assert call_kwargs["Page_Load_Timeout"] is None
|
||||
|
||||
# Check that warnings were logged
|
||||
assert mock_logger.warning.call_count == 3
|
||||
mock_logger.warning.assert_any_call(
|
||||
"Invalid timeout value for timeout: invalid, setting to None"
|
||||
)
|
||||
mock_logger.warning.assert_any_call(
|
||||
"Invalid timeout value for connect_timeout: not_a_number, setting to None"
|
||||
)
|
||||
mock_logger.warning.assert_any_call(
|
||||
"Invalid timeout value for Page_Load_Timeout: abc123, setting to None"
|
||||
)
|
||||
|
||||
@patch("superset.utils.webdriver.app")
|
||||
@patch("superset.utils.webdriver.chrome")
|
||||
def test_non_timeout_config_preserved(self, mock_chrome, mock_app_patch, mock_app):
|
||||
"""Test that non-timeout configuration values are preserved."""
|
||||
mock_app_patch.config = {
|
||||
"WEBDRIVER_TYPE": "chrome",
|
||||
"WEBDRIVER_OPTION_ARGS": [],
|
||||
"SCREENSHOT_LOCATE_WAIT": 10,
|
||||
"SCREENSHOT_LOAD_WAIT": 10,
|
||||
"WEBDRIVER_WINDOW": {},
|
||||
"WEBDRIVER_CONFIGURATION": {
|
||||
"timeout": "30",
|
||||
"some_other_option": "value",
|
||||
"another_option": 123,
|
||||
"boolean_option": True,
|
||||
},
|
||||
}
|
||||
|
||||
mock_driver_class = MagicMock()
|
||||
mock_chrome.webdriver.WebDriver = mock_driver_class
|
||||
mock_chrome.service.Service = MagicMock()
|
||||
mock_options = MagicMock()
|
||||
mock_options.add_argument = MagicMock()
|
||||
mock_chrome.options.Options = MagicMock(return_value=mock_options)
|
||||
|
||||
driver = WebDriverSelenium(driver_type="chrome")
|
||||
driver.create()
|
||||
|
||||
# Check that all config values are passed through
|
||||
mock_driver_class.assert_called_once()
|
||||
call_kwargs = mock_driver_class.call_args.kwargs
|
||||
assert call_kwargs["timeout"] == 30.0
|
||||
assert call_kwargs["some_other_option"] == "value"
|
||||
assert call_kwargs["another_option"] == 123
|
||||
assert call_kwargs["boolean_option"] is True
|
||||
|
||||
@patch("superset.utils.webdriver.app")
|
||||
@patch("superset.utils.webdriver.chrome")
|
||||
def test_timeout_key_case_insensitive(self, mock_chrome, mock_app_patch, mock_app):
|
||||
"""Test that timeout detection is case-insensitive."""
|
||||
mock_app_patch.config = {
|
||||
"WEBDRIVER_TYPE": "chrome",
|
||||
"WEBDRIVER_OPTION_ARGS": [],
|
||||
"SCREENSHOT_LOCATE_WAIT": 10,
|
||||
"SCREENSHOT_LOAD_WAIT": 10,
|
||||
"WEBDRIVER_WINDOW": {},
|
||||
"WEBDRIVER_CONFIGURATION": {
|
||||
"TIMEOUT": "10",
|
||||
"Connect_Timeout": "20",
|
||||
"SOCKET_TIMEOUT": "30",
|
||||
"connection_timeout_ms": "5000", # Contains 'connection_timeout'
|
||||
},
|
||||
}
|
||||
|
||||
mock_driver_class = MagicMock()
|
||||
mock_chrome.webdriver.WebDriver = mock_driver_class
|
||||
mock_chrome.service.Service = MagicMock()
|
||||
mock_options = MagicMock()
|
||||
mock_options.add_argument = MagicMock()
|
||||
mock_chrome.options.Options = MagicMock(return_value=mock_options)
|
||||
|
||||
driver = WebDriverSelenium(driver_type="chrome")
|
||||
driver.create()
|
||||
|
||||
# Check that all timeout values are converted to float
|
||||
mock_driver_class.assert_called_once()
|
||||
call_kwargs = mock_driver_class.call_args.kwargs
|
||||
assert call_kwargs["TIMEOUT"] == 10.0
|
||||
assert call_kwargs["Connect_Timeout"] == 20.0
|
||||
assert call_kwargs["SOCKET_TIMEOUT"] == 30.0
|
||||
assert call_kwargs["connection_timeout_ms"] == 5000.0
|
||||
|
||||
@patch("superset.utils.webdriver.app")
|
||||
@patch("superset.utils.webdriver.chrome")
|
||||
def test_empty_webdriver_config(self, mock_chrome, mock_app_patch, mock_app):
|
||||
"""Test handling of empty webdriver configuration."""
|
||||
mock_app_patch.config = {
|
||||
"WEBDRIVER_TYPE": "chrome",
|
||||
"WEBDRIVER_OPTION_ARGS": [],
|
||||
"SCREENSHOT_LOCATE_WAIT": 10,
|
||||
"SCREENSHOT_LOAD_WAIT": 10,
|
||||
"WEBDRIVER_WINDOW": {},
|
||||
"WEBDRIVER_CONFIGURATION": {},
|
||||
}
|
||||
|
||||
mock_driver_class = MagicMock()
|
||||
mock_chrome.webdriver.WebDriver = mock_driver_class
|
||||
mock_chrome.service.Service = MagicMock()
|
||||
mock_options = MagicMock()
|
||||
mock_options.add_argument = MagicMock()
|
||||
mock_chrome.options.Options = MagicMock(return_value=mock_options)
|
||||
|
||||
driver = WebDriverSelenium(driver_type="chrome")
|
||||
driver.create()
|
||||
|
||||
# Should create driver without errors
|
||||
mock_driver_class.assert_called_once()
|
||||
Reference in New Issue
Block a user