From 80974e737ab8fede5958e07972c373c48a143757 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Thu, 11 Nov 2021 11:28:11 -0800 Subject: [PATCH 1/3] add feature flag for retina screenshot support --- superset/config.py | 32 ++++++++++++++++++++++++-------- superset/utils/webdriver.py | 28 +++++++++++++++++++++------- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/superset/config.py b/superset/config.py index 4b571dad27a2a..27ee7fc5e32f4 100644 --- a/superset/config.py +++ b/superset/config.py @@ -209,7 +209,8 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # Use all X-Forwarded headers when ENABLE_PROXY_FIX is True. # When proxying to a different port, set "x_port" to 0 to avoid downstream issues. ENABLE_PROXY_FIX = False -PROXY_FIX_CONFIG = {"x_for": 1, "x_proto": 1, "x_host": 1, "x_port": 1, "x_prefix": 1} +PROXY_FIX_CONFIG = {"x_for": 1, "x_proto": 1, + "x_host": 1, "x_port": 1, "x_prefix": 1} # ------------------------------ # GLOBALS FOR APP Builder @@ -410,12 +411,13 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # This could cause the server to run out of memory or compute. "ALLOW_FULL_CSV_EXPORT": False, "UX_BETA": False, + "SCREENSHOTS_USE_RETINA_HIRES": False } # Feature flags may also be set via 'SUPERSET_FEATURE_' prefixed environment vars. DEFAULT_FEATURE_FLAGS.update( { - k[len("SUPERSET_FEATURE_") :]: parse_boolean_string(v) + k[len("SUPERSET_FEATURE_"):]: parse_boolean_string(v) for k, v in os.environ.items() if re.search(r"^SUPERSET_FEATURE_\w+", k) } @@ -438,7 +440,8 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # if hasattr(g, "user") and g.user.is_active: # feature_flags_dict['some_feature'] = g.user and g.user.get_id() == 5 # return feature_flags_dict -GET_FEATURE_FLAGS_FUNC: Optional[Callable[[Dict[str, bool]], Dict[str, bool]]] = None +GET_FEATURE_FLAGS_FUNC: Optional[Callable[[ + Dict[str, bool]], Dict[str, bool]]] = None # A function that receives a feature flag name and an optional default value. # Has a similar utility to GET_FEATURE_FLAGS_FUNC but it's useful to not force the # evaluation of all feature flags when just evaluating a single one. @@ -848,6 +851,8 @@ class CeleryConfig: # pylint: disable=too-few-public-methods CSV_TO_HIVE_UPLOAD_DIRECTORY = "EXTERNAL_HIVE_TABLES/" # Function that creates upload directory dynamically based on the # database used, user and schema provided. + + def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( # pylint: disable=invalid-name database: "Database", user: "models.User", # pylint: disable=unused-argument @@ -951,7 +956,10 @@ def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( # pylint: disable=invalid-name # Provide a callable that receives a tracking_url and returns another # URL. This is used to translate internal Hadoop job tracker URL # into a proxied one -TRACKING_URL_TRANSFORMER = lambda x: x + + +def TRACKING_URL_TRANSFORMER(x): return x + # Interval between consecutive polls when using Hive Engine HIVE_POLL_INTERVAL = int(timedelta(seconds=5).total_seconds()) @@ -993,6 +1001,8 @@ def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( # pylint: disable=invalid-name # def SQL_QUERY_MUTATOR(sql, user_name, security_manager, database): # dttm = datetime.now().isoformat() # return f"-- [SQL LAB] {username} {dttm}\n{sql}" + + def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument sql: str, user_name: Optional[str], @@ -1026,7 +1036,8 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument ALERT_REPORTS_WORKING_TIME_OUT_LAG = int(timedelta(seconds=10).total_seconds()) # if ALERT_REPORTS_WORKING_TIME_OUT_KILL is True, set a celery hard timeout # Equal to working timeout + ALERT_REPORTS_WORKING_SOFT_TIME_OUT_LAG -ALERT_REPORTS_WORKING_SOFT_TIME_OUT_LAG = int(timedelta(seconds=1).total_seconds()) +ALERT_REPORTS_WORKING_SOFT_TIME_OUT_LAG = int( + timedelta(seconds=1).total_seconds()) # If set to true no notification is sent, the worker will just log a message. # Useful for debugging ALERT_REPORTS_NOTIFICATION_DRY_RUN = False @@ -1215,7 +1226,10 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument # to allow mutating the object with this callback. # This can be used to set any properties of the object based on naming # conventions and such. You can find examples in the tests. -SQLA_TABLE_MUTATOR = lambda table: table + + +def SQLA_TABLE_MUTATOR(table): return table + # Global async query config options. # Requires GLOBAL_ASYNC_QUERIES feature flag to be enabled. @@ -1302,9 +1316,11 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument elif importlib.util.find_spec("superset_config") and not is_test(): try: import superset_config # pylint: disable=import-error - from superset_config import * # type: ignore # pylint: disable=import-error,wildcard-import,unused-wildcard-import + # type: ignore # pylint: disable=import-error,wildcard-import,unused-wildcard-import + from superset_config import * - print(f"Loaded your LOCAL configuration at [{superset_config.__file__}]") + print( + f"Loaded your LOCAL configuration at [{superset_config.__file__}]") except Exception: logger.exception("Found but failed to import local superset_config") raise diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py index 8ce7ee7c25de6..dce046d66e01d 100644 --- a/superset/utils/webdriver.py +++ b/superset/utils/webdriver.py @@ -17,6 +17,8 @@ import logging from enum import Enum +from superset.extensions import feature_flag_manager +from superset import config from time import sleep from typing import Any, Dict, Optional, Tuple, TYPE_CHECKING @@ -27,7 +29,7 @@ TimeoutException, WebDriverException, ) -from selenium.webdriver import chrome, firefox +from selenium.webdriver import chrome, firefox, FirefoxProfile from selenium.webdriver.common.by import By from selenium.webdriver.remote.webdriver import WebDriver from selenium.webdriver.support import expected_conditions as EC @@ -63,19 +65,28 @@ def create(self) -> WebDriver: if self._driver_type == "firefox": driver_class = firefox.webdriver.WebDriver options = firefox.options.Options() + profile = FirefoxProfile() + if feature_flag_manager.is_feature_enabled("SCREENSHOTS_USE_RETINA_HIRES"): + profile.set_preference("layout.css.devPixelsPerPx", "2") + kwargs: Dict[Any, Any] = dict( + options=options, firefox_profile=profile) elif self._driver_type == "chrome": driver_class = chrome.webdriver.WebDriver options = chrome.options.Options() - options.add_argument(f"--window-size={self._window[0]},{self._window[1]}") + if feature_flag_manager.is_feature_enabled("SCREENSHOTS_USE_RETINA_HIRES"): + options.add_argument("--force-device-scale-factor=2") + options.add_argument( + f"--window-size={self._window[0]},{self._window[1]}") + kwargs: Dict[Any, Any] = dict(options=options) else: - raise Exception(f"Webdriver name ({self._driver_type}) not supported") + raise Exception( + f"Webdriver name ({self._driver_type}) not supported") # Prepare args for the webdriver init # Add additional configured options for arg in current_app.config["WEBDRIVER_OPTION_ARGS"]: options.add_argument(arg) - kwargs: Dict[Any, Any] = dict(options=options) kwargs.update(current_app.config["WEBDRIVER_CONFIGURATION"]) logger.info("Init selenium driver") @@ -135,12 +146,14 @@ def get_screenshot( selenium_animation_wait = current_app.config[ "SCREENSHOT_SELENIUM_ANIMATION_WAIT" ] - logger.debug("Wait %i seconds for chart animation", selenium_animation_wait) + logger.debug("Wait %i seconds for chart animation", + selenium_animation_wait) sleep(selenium_animation_wait) logger.info("Taking a PNG screenshot of url %s", url) img = element.screenshot_as_png except TimeoutException: - logger.warning("Selenium timed out requesting url %s", url, exc_info=True) + logger.warning( + "Selenium timed out requesting url %s", url, exc_info=True) img = element.screenshot_as_png except StaleElementReferenceException: logger.error( @@ -151,5 +164,6 @@ def get_screenshot( except WebDriverException as ex: logger.error(ex, exc_info=True) finally: - self.destroy(driver, current_app.config["SCREENSHOT_SELENIUM_RETRIES"]) + self.destroy( + driver, current_app.config["SCREENSHOT_SELENIUM_RETRIES"]) return img From 42027f11b5eb1136b2e3797321d638263c0e6671 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Fri, 12 Nov 2021 09:38:06 -0800 Subject: [PATCH 2/3] use config for pixel density --- superset/config.py | 34 +++++++++++++++------------------- superset/utils/webdriver.py | 25 ++++++++++++++++--------- 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/superset/config.py b/superset/config.py index 27ee7fc5e32f4..047afec32e20e 100644 --- a/superset/config.py +++ b/superset/config.py @@ -209,8 +209,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # Use all X-Forwarded headers when ENABLE_PROXY_FIX is True. # When proxying to a different port, set "x_port" to 0 to avoid downstream issues. ENABLE_PROXY_FIX = False -PROXY_FIX_CONFIG = {"x_for": 1, "x_proto": 1, - "x_host": 1, "x_port": 1, "x_prefix": 1} +PROXY_FIX_CONFIG = {"x_for": 1, "x_proto": 1, "x_host": 1, "x_port": 1, "x_prefix": 1} # ------------------------------ # GLOBALS FOR APP Builder @@ -411,13 +410,12 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # This could cause the server to run out of memory or compute. "ALLOW_FULL_CSV_EXPORT": False, "UX_BETA": False, - "SCREENSHOTS_USE_RETINA_HIRES": False } # Feature flags may also be set via 'SUPERSET_FEATURE_' prefixed environment vars. DEFAULT_FEATURE_FLAGS.update( { - k[len("SUPERSET_FEATURE_"):]: parse_boolean_string(v) + k[len("SUPERSET_FEATURE_") :]: parse_boolean_string(v) for k, v in os.environ.items() if re.search(r"^SUPERSET_FEATURE_\w+", k) } @@ -440,8 +438,7 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # if hasattr(g, "user") and g.user.is_active: # feature_flags_dict['some_feature'] = g.user and g.user.get_id() == 5 # return feature_flags_dict -GET_FEATURE_FLAGS_FUNC: Optional[Callable[[ - Dict[str, bool]], Dict[str, bool]]] = None +GET_FEATURE_FLAGS_FUNC: Optional[Callable[[Dict[str, bool]], Dict[str, bool]]] = None # A function that receives a feature flag name and an optional default value. # Has a similar utility to GET_FEATURE_FLAGS_FUNC but it's useful to not force the # evaluation of all feature flags when just evaluating a single one. @@ -956,9 +953,8 @@ def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( # pylint: disable=invalid-name # Provide a callable that receives a tracking_url and returns another # URL. This is used to translate internal Hadoop job tracker URL # into a proxied one - - -def TRACKING_URL_TRANSFORMER(x): return x +def TRACKING_URL_TRANSFORMER(x): # pylint: disable=invalid-name + return x # Interval between consecutive polls when using Hive Engine @@ -1001,8 +997,6 @@ def TRACKING_URL_TRANSFORMER(x): return x # def SQL_QUERY_MUTATOR(sql, user_name, security_manager, database): # dttm = datetime.now().isoformat() # return f"-- [SQL LAB] {username} {dttm}\n{sql}" - - def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument sql: str, user_name: Optional[str], @@ -1036,8 +1030,7 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument ALERT_REPORTS_WORKING_TIME_OUT_LAG = int(timedelta(seconds=10).total_seconds()) # if ALERT_REPORTS_WORKING_TIME_OUT_KILL is True, set a celery hard timeout # Equal to working timeout + ALERT_REPORTS_WORKING_SOFT_TIME_OUT_LAG -ALERT_REPORTS_WORKING_SOFT_TIME_OUT_LAG = int( - timedelta(seconds=1).total_seconds()) +ALERT_REPORTS_WORKING_SOFT_TIME_OUT_LAG = int(timedelta(seconds=1).total_seconds()) # If set to true no notification is sent, the worker will just log a message. # Useful for debugging ALERT_REPORTS_NOTIFICATION_DRY_RUN = False @@ -1096,7 +1089,11 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument WEBDRIVER_TYPE = "firefox" # Window size - this will impact the rendering of the data -WEBDRIVER_WINDOW = {"dashboard": (1600, 2000), "slice": (3000, 1200)} +WEBDRIVER_WINDOW = { + "dashboard": (1600, 2000), + "slice": (3000, 1200), + "pixel_density": 1, +} # An optional override to the default auth hook used to provide auth to the # offline webdriver @@ -1226,9 +1223,8 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument # to allow mutating the object with this callback. # This can be used to set any properties of the object based on naming # conventions and such. You can find examples in the tests. - - -def SQLA_TABLE_MUTATOR(table): return table +def SQLA_TABLE_MUTATOR(table): # pylint: disable=invalid-name + return table # Global async query config options. @@ -1316,11 +1312,11 @@ def SQLA_TABLE_MUTATOR(table): return table elif importlib.util.find_spec("superset_config") and not is_test(): try: import superset_config # pylint: disable=import-error + # type: ignore # pylint: disable=import-error,wildcard-import,unused-wildcard-import from superset_config import * - print( - f"Loaded your LOCAL configuration at [{superset_config.__file__}]") + print(f"Loaded your LOCAL configuration at [{superset_config.__file__}]") except Exception: logger.exception("Found but failed to import local superset_config") raise diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py index dce046d66e01d..02396810aac7e 100644 --- a/superset/utils/webdriver.py +++ b/superset/utils/webdriver.py @@ -17,8 +17,6 @@ import logging from enum import Enum -from superset.extensions import feature_flag_manager -from superset import config from time import sleep from typing import Any, Dict, Optional, Tuple, TYPE_CHECKING @@ -54,7 +52,9 @@ class DashboardStandaloneMode(Enum): class WebDriverProxy: def __init__( - self, driver_type: str, window: Optional[WindowSize] = None, + self, + driver_type: str, + window: Optional[WindowSize] = None, ): self._driver_type = driver_type self._window: WindowSize = window or (800, 600) @@ -62,22 +62,26 @@ def __init__( self._screenshot_load_wait = current_app.config["SCREENSHOT_LOAD_WAIT"] def create(self) -> WebDriver: + pixel_density = current_app.config["WEBDRIVER_WINDOW"].get( + "pixel_density", 1) if self._driver_type == "firefox": driver_class = firefox.webdriver.WebDriver options = firefox.options.Options() profile = FirefoxProfile() - if feature_flag_manager.is_feature_enabled("SCREENSHOTS_USE_RETINA_HIRES"): - profile.set_preference("layout.css.devPixelsPerPx", "2") + profile.set_preference( + "layout.css.devPixelsPerPx", + str(pixel_density), + ) kwargs: Dict[Any, Any] = dict( options=options, firefox_profile=profile) elif self._driver_type == "chrome": driver_class = chrome.webdriver.WebDriver options = chrome.options.Options() - if feature_flag_manager.is_feature_enabled("SCREENSHOTS_USE_RETINA_HIRES"): - options.add_argument("--force-device-scale-factor=2") + options.add_argument( + f"--force-device-scale-factor={pixel_density}") options.add_argument( f"--window-size={self._window[0]},{self._window[1]}") - kwargs: Dict[Any, Any] = dict(options=options) + kwargs = dict(options=options) else: raise Exception( f"Webdriver name ({self._driver_type}) not supported") @@ -113,7 +117,10 @@ def destroy(driver: WebDriver, tries: int = 2) -> None: pass def get_screenshot( - self, url: str, element_name: str, user: "User", + self, + url: str, + element_name: str, + user: "User", ) -> Optional[bytes]: params = {"standalone": DashboardStandaloneMode.REPORT.value} req = PreparedRequest() From 8604cad704e1136771da80d90acd87f90df046b4 Mon Sep 17 00:00:00 2001 From: Elizabeth Thompson Date: Mon, 15 Nov 2021 09:59:36 -0800 Subject: [PATCH 3/3] run black --- superset/config.py | 14 +++---------- superset/utils/webdriver.py | 40 ++++++++++--------------------------- 2 files changed, 14 insertions(+), 40 deletions(-) diff --git a/superset/config.py b/superset/config.py index 047afec32e20e..05d7b8ecd706e 100644 --- a/superset/config.py +++ b/superset/config.py @@ -848,8 +848,6 @@ class CeleryConfig: # pylint: disable=too-few-public-methods CSV_TO_HIVE_UPLOAD_DIRECTORY = "EXTERNAL_HIVE_TABLES/" # Function that creates upload directory dynamically based on the # database used, user and schema provided. - - def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( # pylint: disable=invalid-name database: "Database", user: "models.User", # pylint: disable=unused-argument @@ -953,9 +951,7 @@ def CSV_TO_HIVE_UPLOAD_DIRECTORY_FUNC( # pylint: disable=invalid-name # Provide a callable that receives a tracking_url and returns another # URL. This is used to translate internal Hadoop job tracker URL # into a proxied one -def TRACKING_URL_TRANSFORMER(x): # pylint: disable=invalid-name - return x - +TRACKING_URL_TRANSFORMER = lambda x: x # Interval between consecutive polls when using Hive Engine HIVE_POLL_INTERVAL = int(timedelta(seconds=5).total_seconds()) @@ -1223,9 +1219,7 @@ def SQL_QUERY_MUTATOR( # pylint: disable=invalid-name,unused-argument # to allow mutating the object with this callback. # This can be used to set any properties of the object based on naming # conventions and such. You can find examples in the tests. -def SQLA_TABLE_MUTATOR(table): # pylint: disable=invalid-name - return table - +SQLA_TABLE_MUTATOR = lambda table: table # Global async query config options. # Requires GLOBAL_ASYNC_QUERIES feature flag to be enabled. @@ -1312,9 +1306,7 @@ def SQLA_TABLE_MUTATOR(table): # pylint: disable=invalid-name elif importlib.util.find_spec("superset_config") and not is_test(): try: import superset_config # pylint: disable=import-error - - # type: ignore # pylint: disable=import-error,wildcard-import,unused-wildcard-import - from superset_config import * + from superset_config import * # type: ignore # pylint: disable=import-error,wildcard-import,unused-wildcard-import print(f"Loaded your LOCAL configuration at [{superset_config.__file__}]") except Exception: diff --git a/superset/utils/webdriver.py b/superset/utils/webdriver.py index 02396810aac7e..9cedc0f934521 100644 --- a/superset/utils/webdriver.py +++ b/superset/utils/webdriver.py @@ -51,40 +51,28 @@ class DashboardStandaloneMode(Enum): class WebDriverProxy: - def __init__( - self, - driver_type: str, - window: Optional[WindowSize] = None, - ): + def __init__(self, driver_type: str, window: Optional[WindowSize] = None): self._driver_type = driver_type self._window: WindowSize = window or (800, 600) self._screenshot_locate_wait = current_app.config["SCREENSHOT_LOCATE_WAIT"] self._screenshot_load_wait = current_app.config["SCREENSHOT_LOAD_WAIT"] def create(self) -> WebDriver: - pixel_density = current_app.config["WEBDRIVER_WINDOW"].get( - "pixel_density", 1) + pixel_density = current_app.config["WEBDRIVER_WINDOW"].get("pixel_density", 1) if self._driver_type == "firefox": driver_class = firefox.webdriver.WebDriver options = firefox.options.Options() profile = FirefoxProfile() - profile.set_preference( - "layout.css.devPixelsPerPx", - str(pixel_density), - ) - kwargs: Dict[Any, Any] = dict( - options=options, firefox_profile=profile) + profile.set_preference("layout.css.devPixelsPerPx", str(pixel_density)) + kwargs: Dict[Any, Any] = dict(options=options, firefox_profile=profile) elif self._driver_type == "chrome": driver_class = chrome.webdriver.WebDriver 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]}") + options.add_argument(f"--force-device-scale-factor={pixel_density}") + options.add_argument(f"--window-size={self._window[0]},{self._window[1]}") kwargs = dict(options=options) else: - raise Exception( - f"Webdriver name ({self._driver_type}) not supported") + raise Exception(f"Webdriver name ({self._driver_type}) not supported") # Prepare args for the webdriver init # Add additional configured options @@ -117,10 +105,7 @@ def destroy(driver: WebDriver, tries: int = 2) -> None: pass def get_screenshot( - self, - url: str, - element_name: str, - user: "User", + self, url: str, element_name: str, user: "User" ) -> Optional[bytes]: params = {"standalone": DashboardStandaloneMode.REPORT.value} req = PreparedRequest() @@ -153,14 +138,12 @@ def get_screenshot( selenium_animation_wait = current_app.config[ "SCREENSHOT_SELENIUM_ANIMATION_WAIT" ] - logger.debug("Wait %i seconds for chart animation", - selenium_animation_wait) + logger.debug("Wait %i seconds for chart animation", selenium_animation_wait) sleep(selenium_animation_wait) logger.info("Taking a PNG screenshot of url %s", url) img = element.screenshot_as_png except TimeoutException: - logger.warning( - "Selenium timed out requesting url %s", url, exc_info=True) + logger.warning("Selenium timed out requesting url %s", url, exc_info=True) img = element.screenshot_as_png except StaleElementReferenceException: logger.error( @@ -171,6 +154,5 @@ def get_screenshot( except WebDriverException as ex: logger.error(ex, exc_info=True) finally: - self.destroy( - driver, current_app.config["SCREENSHOT_SELENIUM_RETRIES"]) + self.destroy(driver, current_app.config["SCREENSHOT_SELENIUM_RETRIES"]) return img