diff --git a/UPDATING.md b/UPDATING.md index 663df68cd9b69..2a9e8992d85ec 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -24,6 +24,7 @@ assists people when migrating to a new version. ## Next +- [31844](https://github.com/apache/superset/pull/31844) The `ALERT_REPORTS_EXECUTE_AS` and `THUMBNAILS_EXECUTE_AS` config parameters have been renamed to `ALERT_REPORTS_EXECUTORS` and `THUMBNAILS_EXECUTORS` respectively. A new config flag `CACHE_WARMUP_EXECUTORS` has also been introduced to be able to control which user is used to execute cache warmup tasks. Finally, the config flag `THUMBNAILS_SELENIUM_USER` has been removed. To use a fixed executor for async tasks, use the new `FixedExecutor` class. See the config and docs for more info on setting up different executor profiles. - [31894](https://github.com/apache/superset/pull/31894) Domain sharding is deprecated in favor of HTTP2. The `SUPERSET_WEBSERVER_DOMAINS` configuration will be removed in the next major version (6.0) - [31774](https://github.com/apache/superset/pull/31774): Fixes the spelling of the `USE-ANALAGOUS-COLORS` feature flag. Please update any scripts/configuration item to use the new/corrected `USE-ANALOGOUS-COLORS` flag spelling. - [31582](https://github.com/apache/superset/pull/31582) Removed the legacy Area, Bar, Event Flow, Heatmap, Histogram, Line, Sankey, and Sankey Loop charts. They were all automatically migrated to their ECharts counterparts with the exception of the Event Flow and Sankey Loop charts which were removed as they were not actively maintained and not widely used. If you were using the Event Flow or Sankey Loop charts, you will need to find an alternative solution. diff --git a/docs/docs/configuration/alerts-reports.mdx b/docs/docs/configuration/alerts-reports.mdx index 293ed3f71ff11..5ff1ef4b819ef 100644 --- a/docs/docs/configuration/alerts-reports.mdx +++ b/docs/docs/configuration/alerts-reports.mdx @@ -177,10 +177,9 @@ By default, Alerts and Reports are executed as the owner of the alert/report obj just change the config as follows (`admin` in this example): ```python -from superset.tasks.types import ExecutorType +from superset.tasks.types import FixedExecutor -THUMBNAIL_SELENIUM_USER = 'admin' -ALERT_REPORTS_EXECUTE_AS = [ExecutorType.SELENIUM] +ALERT_REPORTS_EXECUTORS = [FixedExecutor("admin")] ``` Please refer to `ExecutorType` in the codebase for other executor types. diff --git a/docs/docs/configuration/cache.mdx b/docs/docs/configuration/cache.mdx index 6d761c56b7113..c0eadca95bfaf 100644 --- a/docs/docs/configuration/cache.mdx +++ b/docs/docs/configuration/cache.mdx @@ -94,10 +94,9 @@ By default thumbnails are rendered per user, and will fall back to the Selenium To always render thumbnails as a fixed user (`admin` in this example), use the following configuration: ```python -from superset.tasks.types import ExecutorType +from superset.tasks.types import FixedExecutor -THUMBNAIL_SELENIUM_USER = "admin" -THUMBNAIL_EXECUTE_AS = [ExecutorType.SELENIUM] +THUMBNAIL_EXECUTORS = [FixedExecutor("admin")] ``` @@ -130,8 +129,6 @@ def init_thumbnail_cache(app: Flask) -> S3Cache: THUMBNAIL_CACHE_CONFIG = init_thumbnail_cache -# Async selenium thumbnail task will use the following user -THUMBNAIL_SELENIUM_USER = "Admin" ``` Using the above example cache keys for dashboards will be `superset_thumb__dashboard__{ID}`. You can diff --git a/superset-frontend/src/components/ListViewCard/index.tsx b/superset-frontend/src/components/ListViewCard/index.tsx index 8d897d5bd32c0..cba0a089792eb 100644 --- a/superset-frontend/src/components/ListViewCard/index.tsx +++ b/superset-frontend/src/components/ListViewCard/index.tsx @@ -153,7 +153,7 @@ interface CardProps { subtitle?: ReactNode; url?: string; linkComponent?: ComponentType; - imgURL?: string; + imgURL?: string | null; imgFallbackURL?: string; imgPosition?: BackgroundPosition; description: string; diff --git a/superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.tsx b/superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.tsx index 0ab150e529a68..42852fef4d900 100644 --- a/superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.tsx +++ b/superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.tsx @@ -174,7 +174,7 @@ const AddSliceCard: FC<{ lastModified?: string; sliceName: string; style?: CSSProperties; - thumbnailUrl?: string; + thumbnailUrl?: string | null; visType: string; }> = ({ datasourceUrl, diff --git a/superset-frontend/src/types/Dashboard.ts b/superset-frontend/src/types/Dashboard.ts index faecc0bc4acf7..38e5e0fe52f0b 100644 --- a/superset-frontend/src/types/Dashboard.ts +++ b/superset-frontend/src/types/Dashboard.ts @@ -24,7 +24,7 @@ export interface Dashboard { slug?: string | null; url: string; dashboard_title: string; - thumbnail_url: string; + thumbnail_url: string | null; published: boolean; css?: string | null; json_metadata?: string | null; diff --git a/superset/commands/report/alert.py b/superset/commands/report/alert.py index d713c45811021..458f78fd3c667 100644 --- a/superset/commands/report/alert.py +++ b/superset/commands/report/alert.py @@ -170,7 +170,7 @@ def _execute_query(self) -> pd.DataFrame: ) executor, username = get_executor( # pylint: disable=unused-variable - executor_types=app.config["ALERT_REPORTS_EXECUTE_AS"], + executors=app.config["ALERT_REPORTS_EXECUTORS"], model=self._report_schedule, ) user = security_manager.find_user(username) diff --git a/superset/commands/report/execute.py b/superset/commands/report/execute.py index 54a2890a96f91..9e69258650dad 100644 --- a/superset/commands/report/execute.py +++ b/superset/commands/report/execute.py @@ -295,7 +295,7 @@ def _get_screenshots(self) -> list[bytes]: :raises: ReportScheduleScreenshotFailedError """ _, username = get_executor( - executor_types=app.config["ALERT_REPORTS_EXECUTE_AS"], + executors=app.config["ALERT_REPORTS_EXECUTORS"], model=self._report_schedule, ) user = security_manager.find_user(username) @@ -360,7 +360,7 @@ def _get_pdf(self) -> bytes: def _get_csv_data(self) -> bytes: url = self._get_url(result_format=ChartDataResultFormat.CSV) _, username = get_executor( - executor_types=app.config["ALERT_REPORTS_EXECUTE_AS"], + executors=app.config["ALERT_REPORTS_EXECUTORS"], model=self._report_schedule, ) user = security_manager.find_user(username) @@ -389,7 +389,7 @@ def _get_embedded_data(self) -> pd.DataFrame: """ url = self._get_url(result_format=ChartDataResultFormat.JSON) _, username = get_executor( - executor_types=app.config["ALERT_REPORTS_EXECUTE_AS"], + executors=app.config["ALERT_REPORTS_EXECUTORS"], model=self._report_schedule, ) user = security_manager.find_user(username) @@ -859,7 +859,7 @@ def run(self) -> None: if not self._model: raise ReportScheduleExecuteUnexpectedError() _, username = get_executor( - executor_types=app.config["ALERT_REPORTS_EXECUTE_AS"], + executors=app.config["ALERT_REPORTS_EXECUTORS"], model=self._model, ) user = security_manager.find_user(username) diff --git a/superset/config.py b/superset/config.py index ae944f3ca11ef..89fbdf35a05a3 100644 --- a/superset/config.py +++ b/superset/config.py @@ -689,6 +689,15 @@ class D3TimeFormat(TypedDict, total=False): # This is merely a default EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = [] +# User used to execute cache warmup tasks +# By default, the cache is warmed up using the primary owner. To fall back to using +# a fixed user (admin in this example), use the following configuration: +# +# from superset.tasks.types import ExecutorType, FixedExecutor +# +# CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER, FixedExecutor("admin")] +CACHE_WARMUP_EXECUTORS = [ExecutorType.OWNER] + # --------------------------------------------------- # Thumbnail config (behind feature flag) # --------------------------------------------------- @@ -696,25 +705,30 @@ class D3TimeFormat(TypedDict, total=False): # user for anonymous users. Similar to Alerts & Reports, thumbnails # can be configured to always be rendered as a fixed user. See # `superset.tasks.types.ExecutorType` for a full list of executor options. -# To always use a fixed user account, use the following configuration: -# THUMBNAIL_EXECUTE_AS = [ExecutorType.SELENIUM] -THUMBNAIL_SELENIUM_USER: str | None = "admin" -THUMBNAIL_EXECUTE_AS = [ExecutorType.CURRENT_USER, ExecutorType.SELENIUM] +# To always use a fixed user account (admin in this example, use the following +# configuration: +# +# from superset.tasks.types import ExecutorType, FixedExecutor +# +# THUMBNAIL_EXECUTORS = [FixedExecutor("admin")] +THUMBNAIL_EXECUTORS = [ExecutorType.CURRENT_USER] # By default, thumbnail digests are calculated based on various parameters in the # chart/dashboard metadata, and in the case of user-specific thumbnails, the # username. To specify a custom digest function, use the following config parameters # to define callbacks that receive # 1. the model (dashboard or chart) -# 2. the executor type (e.g. ExecutorType.SELENIUM) +# 2. the executor type (e.g. ExecutorType.FIXED_USER) # 3. the executor's username (note, this is the executor as defined by -# `THUMBNAIL_EXECUTE_AS`; the executor is only equal to the currently logged in +# `THUMBNAIL_EXECUTORS`; the executor is only equal to the currently logged in # user if the executor type is equal to `ExecutorType.CURRENT_USER`) # and return the final digest string: THUMBNAIL_DASHBOARD_DIGEST_FUNC: ( - None | (Callable[[Dashboard, ExecutorType, str], str]) + Callable[[Dashboard, ExecutorType, str], str | None] | None ) = None -THUMBNAIL_CHART_DIGEST_FUNC: Callable[[Slice, ExecutorType, str], str] | None = None +THUMBNAIL_CHART_DIGEST_FUNC: Callable[[Slice, ExecutorType, str], str | None] | None = ( + None +) THUMBNAIL_CACHE_CONFIG: CacheConfig = { "CACHE_TYPE": "NullCache", @@ -1421,16 +1435,19 @@ def EMAIL_HEADER_MUTATOR( # pylint: disable=invalid-name,unused-argument # noq # # To first try to execute as the creator in the owners list (if present), then fall # back to the creator, then the last modifier in the owners list (if present), then the -# last modifier, then an owner and finally `THUMBNAIL_SELENIUM_USER`, set as follows: -# ALERT_REPORTS_EXECUTE_AS = [ +# last modifier, then an owner and finally the "admin" user, set as follows: +# +# from superset.tasks.types import ExecutorType, FixedExecutor +# +# ALERT_REPORTS_EXECUTORS = [ # ExecutorType.CREATOR_OWNER, # ExecutorType.CREATOR, # ExecutorType.MODIFIER_OWNER, # ExecutorType.MODIFIER, # ExecutorType.OWNER, -# ExecutorType.SELENIUM, +# FixedExecutor("admin"), # ] -ALERT_REPORTS_EXECUTE_AS: list[ExecutorType] = [ExecutorType.OWNER] +ALERT_REPORTS_EXECUTORS: list[ExecutorType] = [ExecutorType.OWNER] # if ALERT_REPORTS_WORKING_TIME_OUT_KILL is True, set a celery hard timeout # Equal to working timeout + ALERT_REPORTS_WORKING_TIME_OUT_LAG ALERT_REPORTS_WORKING_TIME_OUT_LAG = int(timedelta(seconds=10).total_seconds()) diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py index 1295f0b206538..5b18e856c93b4 100644 --- a/superset/dashboards/schemas.py +++ b/superset/dashboards/schemas.py @@ -212,7 +212,7 @@ class DashboardGetResponseSchema(Schema): dashboard_title = fields.String( metadata={"description": dashboard_title_description} ) - thumbnail_url = fields.String() + thumbnail_url = fields.String(allow_none=True) published = fields.Boolean() css = fields.String(metadata={"description": css_description}) json_metadata = fields.String(metadata={"description": json_metadata_description}) diff --git a/superset/models/dashboard.py b/superset/models/dashboard.py index 3af3a63ab7668..5b8675a6a0881 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -225,16 +225,19 @@ def dashboard_link(self) -> Markup: return Markup(f'{title}') @property - def digest(self) -> str: + def digest(self) -> str | None: return get_dashboard_digest(self) @property - def thumbnail_url(self) -> str: + def thumbnail_url(self) -> str | None: """ Returns a thumbnail URL with a HEX digest. We want to avoid browser cache if the dashboard has changed """ - return f"/api/v1/dashboard/{self.id}/thumbnail/{self.digest}/" + if digest := self.digest: + return f"/api/v1/dashboard/{self.id}/thumbnail/{digest}/" + + return None @property def changed_by_name(self) -> str: diff --git a/superset/models/slice.py b/superset/models/slice.py index 1e6daa8321112..8795d186ef45f 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -247,16 +247,19 @@ def data(self) -> dict[str, Any]: } @property - def digest(self) -> str: + def digest(self) -> str | None: return get_chart_digest(self) @property - def thumbnail_url(self) -> str: + def thumbnail_url(self) -> str | None: """ Returns a thumbnail URL with a HEX digest. We want to avoid browser cache if the dashboard has changed """ - return f"/api/v1/chart/{self.id}/thumbnail/{self.digest}/" + if digest := self.digest: + return f"/api/v1/chart/{self.id}/thumbnail/{digest}/" + + return None @property def json_data(self) -> str: diff --git a/superset/tasks/cache.py b/superset/tasks/cache.py index b1eaff58bd6c4..1c8012ff839a0 100644 --- a/superset/tasks/cache.py +++ b/superset/tasks/cache.py @@ -14,22 +14,26 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from __future__ import annotations + import logging -from typing import Any, Optional, Union +from typing import Any, Optional, TypedDict, Union from urllib import request from urllib.error import URLError from celery.beat import SchedulingError from celery.utils.log import get_task_logger +from flask import current_app from sqlalchemy import and_, func -from superset import app, db, security_manager +from superset import db, security_manager from superset.extensions import celery_app from superset.models.core import Log from superset.models.dashboard import Dashboard from superset.models.slice import Slice from superset.tags.models import Tag, TaggedObject -from superset.tasks.utils import fetch_csrf_token +from superset.tasks.exceptions import ExecutorNotFoundError, InvalidExecutorError +from superset.tasks.utils import fetch_csrf_token, get_executor from superset.utils import json from superset.utils.date_parser import parse_human_datetime from superset.utils.machine_auth import MachineAuthProvider @@ -39,19 +43,38 @@ logger.setLevel(logging.INFO) -def get_payload(chart: Slice, dashboard: Optional[Dashboard] = None) -> dict[str, int]: - """Return payload for warming up a given chart/table cache.""" - payload = {"chart_id": chart.id} +class CacheWarmupPayload(TypedDict, total=False): + chart_id: int + dashboard_id: int | None + + +class CacheWarmupTask(TypedDict): + payload: CacheWarmupPayload + username: str | None + + +def get_task(chart: Slice, dashboard: Optional[Dashboard] = None) -> CacheWarmupTask: + """Return task for warming up a given chart/table cache.""" + executors = current_app.config["CACHE_WARMUP_EXECUTORS"] + payload: CacheWarmupPayload = {"chart_id": chart.id} if dashboard: payload["dashboard_id"] = dashboard.id - return payload + + username: str | None + try: + executor = get_executor(executors, chart) + username = executor[1] + except (ExecutorNotFoundError, InvalidExecutorError): + username = None + + return {"payload": payload, "username": username} class Strategy: # pylint: disable=too-few-public-methods """ A cache warm up strategy. - Each strategy defines a `get_payloads` method that returns a list of payloads to + Each strategy defines a `get_tasks` method that returns a list of tasks to send to the `/api/v1/chart/warm_up_cache` endpoint. Strategies can be configured in `superset/config.py`: @@ -73,8 +96,8 @@ class Strategy: # pylint: disable=too-few-public-methods def __init__(self) -> None: pass - def get_payloads(self) -> list[dict[str, int]]: - raise NotImplementedError("Subclasses must implement get_payloads!") + def get_tasks(self) -> list[CacheWarmupTask]: + raise NotImplementedError("Subclasses must implement get_tasks!") class DummyStrategy(Strategy): # pylint: disable=too-few-public-methods @@ -95,8 +118,8 @@ class DummyStrategy(Strategy): # pylint: disable=too-few-public-methods name = "dummy" - def get_payloads(self) -> list[dict[str, int]]: - return [get_payload(chart) for chart in db.session.query(Slice).all()] + def get_tasks(self) -> list[CacheWarmupTask]: + return [get_task(chart) for chart in db.session.query(Slice).all()] class TopNDashboardsStrategy(Strategy): # pylint: disable=too-few-public-methods @@ -124,7 +147,7 @@ def __init__(self, top_n: int = 5, since: str = "7 days ago") -> None: self.top_n = top_n self.since = parse_human_datetime(since) if since else None - def get_payloads(self) -> list[dict[str, int]]: + def get_tasks(self) -> list[CacheWarmupTask]: records = ( db.session.query(Log.dashboard_id, func.count(Log.dashboard_id)) .filter(and_(Log.dashboard_id.isnot(None), Log.dttm >= self.since)) @@ -139,7 +162,7 @@ def get_payloads(self) -> list[dict[str, int]]: ) return [ - get_payload(chart, dashboard) + get_task(chart, dashboard) for dashboard in dashboards for chart in dashboard.slices ] @@ -167,8 +190,8 @@ def __init__(self, tags: Optional[list[str]] = None) -> None: super().__init__() self.tags = tags or [] - def get_payloads(self) -> list[dict[str, int]]: - payloads = [] + def get_tasks(self) -> list[CacheWarmupTask]: + tasks = [] tags = db.session.query(Tag).filter(Tag.name.in_(self.tags)).all() tag_ids = [tag.id for tag in tags] @@ -189,7 +212,7 @@ def get_payloads(self) -> list[dict[str, int]]: ) for dashboard in tagged_dashboards: for chart in dashboard.slices: - payloads.append(get_payload(chart)) + tasks.append(get_task(chart)) # add charts that are tagged tagged_objects = ( @@ -205,9 +228,9 @@ def get_payloads(self) -> list[dict[str, int]]: chart_ids = [tagged_object.object_id for tagged_object in tagged_objects] tagged_charts = db.session.query(Slice).filter(Slice.id.in_(chart_ids)) for chart in tagged_charts: - payloads.append(get_payload(chart)) + tasks.append(get_task(chart)) - return payloads + return tasks strategies = [DummyStrategy, TopNDashboardsStrategy, DashboardTagsStrategy] @@ -284,22 +307,25 @@ def cache_warmup( logger.exception(message) return message - user = security_manager.get_user_by_username(app.config["THUMBNAIL_SELENIUM_USER"]) - cookies = MachineAuthProvider.get_auth_cookies(user) - headers = { - "Cookie": f"session={cookies.get('session', '')}", - "Content-Type": "application/json", - } - results: dict[str, list[str]] = {"scheduled": [], "errors": []} - for payload in strategy.get_payloads(): - try: - payload = json.dumps(payload) - logger.info("Scheduling %s", payload) - fetch_url.delay(payload, headers) - results["scheduled"].append(payload) - except SchedulingError: - logger.exception("Error scheduling fetch_url for payload: %s", payload) - results["errors"].append(payload) + for task in strategy.get_tasks(): + username = task["username"] + payload = json.dumps(task["payload"]) + if username: + try: + user = security_manager.get_user_by_username(username) + cookies = MachineAuthProvider.get_auth_cookies(user) + headers = { + "Cookie": f"session={cookies.get('session', '')}", + "Content-Type": "application/json", + } + logger.info("Scheduling %s", payload) + fetch_url.delay(payload, headers) + results["scheduled"].append(payload) + except SchedulingError: + logger.exception("Error scheduling fetch_url for payload: %s", payload) + results["errors"].append(payload) + else: + logger.warn("Executor not found for %s", payload) return results diff --git a/superset/tasks/exceptions.py b/superset/tasks/exceptions.py index 6698661754e5e..19a97ea658b81 100644 --- a/superset/tasks/exceptions.py +++ b/superset/tasks/exceptions.py @@ -22,3 +22,7 @@ class ExecutorNotFoundError(SupersetException): message = _("Scheduled task executor not found") + + +class InvalidExecutorError(SupersetException): + message = _("Invalid executor type") diff --git a/superset/tasks/thumbnails.py b/superset/tasks/thumbnails.py index dd9b5065dce34..3b0b47dbb368f 100644 --- a/superset/tasks/thumbnails.py +++ b/superset/tasks/thumbnails.py @@ -55,7 +55,7 @@ def cache_chart_thumbnail( url = get_url_path("Superset.slice", slice_id=chart.id) logger.info("Caching chart: %s", url) _, username = get_executor( - executor_types=current_app.config["THUMBNAIL_EXECUTE_AS"], + executors=current_app.config["THUMBNAIL_EXECUTORS"], model=chart, current_user=current_user, ) @@ -92,7 +92,7 @@ def cache_dashboard_thumbnail( logger.info("Caching dashboard: %s", url) _, username = get_executor( - executor_types=current_app.config["THUMBNAIL_EXECUTE_AS"], + executors=current_app.config["THUMBNAIL_EXECUTORS"], model=dashboard, current_user=current_user, ) @@ -135,7 +135,7 @@ def cache_dashboard_screenshot( # pylint: disable=too-many-arguments current_user = security_manager.get_guest_user_from_token(guest_token) else: _, exec_username = get_executor( - executor_types=current_app.config["THUMBNAIL_EXECUTE_AS"], + executors=current_app.config["THUMBNAIL_EXECUTORS"], model=dashboard, current_user=username, ) diff --git a/superset/tasks/types.py b/superset/tasks/types.py index 84a3e7b01f26c..8f2f76b40528f 100644 --- a/superset/tasks/types.py +++ b/superset/tasks/types.py @@ -14,18 +14,25 @@ # KIND, either express or implied. See the License for the # specific language governing permissions and limitations # under the License. +from typing import NamedTuple + from superset.utils.backports import StrEnum +class FixedExecutor(NamedTuple): + username: str + + class ExecutorType(StrEnum): """ - Which user should scheduled tasks be executed as. Used as follows: + Which user should async tasks be executed as. Used as follows: For Alerts & Reports: the "model" refers to the AlertSchedule object For Thumbnails: The "model" refers to the Slice or Dashboard object """ - # See the THUMBNAIL_SELENIUM_USER config parameter - SELENIUM = "selenium" + # A fixed user account. Note that for assigning a fixed user you should use the + # FixedExecutor class. + FIXED_USER = "fixed_user" # The creator of the model CREATOR = "creator" # The creator of the model, if found in the owners list @@ -41,3 +48,10 @@ class ExecutorType(StrEnum): # user. If the modifier is not found, returns the creator if found in the owners # list. Finally, if neither are present, returns the first user in the owners list. OWNER = "owner" + + +Executor = FixedExecutor | ExecutorType + + +# Alias type to represent the executor that was chosen from a list of Executors +ChosenExecutor = tuple[ExecutorType, str] diff --git a/superset/tasks/utils.py b/superset/tasks/utils.py index 4815b70343f57..845ea2b5fc906 100644 --- a/superset/tasks/utils.py +++ b/superset/tasks/utils.py @@ -23,10 +23,10 @@ from urllib import request from celery.utils.log import get_task_logger -from flask import current_app, g +from flask import g -from superset.tasks.exceptions import ExecutorNotFoundError -from superset.tasks.types import ExecutorType +from superset.tasks.exceptions import ExecutorNotFoundError, InvalidExecutorError +from superset.tasks.types import ChosenExecutor, Executor, ExecutorType, FixedExecutor from superset.utils import json from superset.utils.urls import get_url_path @@ -42,56 +42,60 @@ # pylint: disable=too-many-branches def get_executor( # noqa: C901 - executor_types: list[ExecutorType], + executors: list[Executor], model: Dashboard | ReportSchedule | Slice, current_user: str | None = None, -) -> tuple[ExecutorType, str]: +) -> ChosenExecutor: """ Extract the user that should be used to execute a scheduled task. Certain executor types extract the user from the underlying object (e.g. CREATOR), the constant Selenium user (SELENIUM), or the user that initiated the request. - :param executor_types: The requested executor type in descending order. When the + :param executors: The requested executor in descending order. When the first user is found it is returned. :param model: The underlying object :param current_user: The username of the user that initiated the task. For thumbnails this is the user that requested the thumbnail, while for alerts and reports this is None (=initiated by Celery). - :return: User to execute the report as - :raises ScheduledTaskExecutorNotFoundError: If no users were found in after - iterating through all entries in `executor_types` + :return: User to execute the execute the async task as. The first element of the + tuple represents the type of the executor, and the second represents the + username of the executor. + :raises ExecutorNotFoundError: If no users were found in after + iterating through all entries in `executors` """ owners = model.owners owner_dict = {owner.id: owner for owner in owners} - for executor_type in executor_types: - if executor_type == ExecutorType.SELENIUM: - return executor_type, current_app.config["THUMBNAIL_SELENIUM_USER"] - if executor_type == ExecutorType.CURRENT_USER and current_user: - return executor_type, current_user - if executor_type == ExecutorType.CREATOR_OWNER: + for executor in executors: + if isinstance(executor, FixedExecutor): + return ExecutorType.FIXED_USER, executor.username + if executor == ExecutorType.FIXED_USER: + raise InvalidExecutorError() + if executor == ExecutorType.CURRENT_USER and current_user: + return executor, current_user + if executor == ExecutorType.CREATOR_OWNER: if (user := model.created_by) and (owner := owner_dict.get(user.id)): - return executor_type, owner.username - if executor_type == ExecutorType.CREATOR: + return executor, owner.username + if executor == ExecutorType.CREATOR: if user := model.created_by: - return executor_type, user.username - if executor_type == ExecutorType.MODIFIER_OWNER: + return executor, user.username + if executor == ExecutorType.MODIFIER_OWNER: if (user := model.changed_by) and (owner := owner_dict.get(user.id)): - return executor_type, owner.username - if executor_type == ExecutorType.MODIFIER: + return executor, owner.username + if executor == ExecutorType.MODIFIER: if user := model.changed_by: - return executor_type, user.username - if executor_type == ExecutorType.OWNER: + return executor, user.username + if executor == ExecutorType.OWNER: owners = model.owners if len(owners) == 1: - return executor_type, owners[0].username + return executor, owners[0].username if len(owners) > 1: if modifier := model.changed_by: if modifier and (user := owner_dict.get(modifier.id)): - return executor_type, user.username + return executor, user.username if creator := model.created_by: if creator and (user := owner_dict.get(creator.id)): - return executor_type, user.username - return executor_type, owners[0].username + return executor, user.username + return executor, owners[0].username raise ExecutorNotFoundError() diff --git a/superset/thumbnails/digest.py b/superset/thumbnails/digest.py index c10e4330cb458..446d06b20d902 100644 --- a/superset/thumbnails/digest.py +++ b/superset/thumbnails/digest.py @@ -23,6 +23,7 @@ from flask import current_app from superset import security_manager +from superset.tasks.exceptions import ExecutorNotFoundError from superset.tasks.types import ExecutorType from superset.tasks.utils import get_current_user, get_executor from superset.utils.core import override_user @@ -89,14 +90,17 @@ def _adjust_string_with_rls( return unique_string -def get_dashboard_digest(dashboard: Dashboard) -> str: +def get_dashboard_digest(dashboard: Dashboard) -> str | None: config = current_app.config - datasources = dashboard.datasources - executor_type, executor = get_executor( - executor_types=config["THUMBNAIL_EXECUTE_AS"], - model=dashboard, - current_user=get_current_user(), - ) + try: + executor_type, executor = get_executor( + executors=config["THUMBNAIL_EXECUTORS"], + model=dashboard, + current_user=get_current_user(), + ) + except ExecutorNotFoundError: + return None + if func := config["THUMBNAIL_DASHBOARD_DIGEST_FUNC"]: return func(dashboard, executor_type, executor) @@ -106,25 +110,29 @@ def get_dashboard_digest(dashboard: Dashboard) -> str: ) unique_string = _adjust_string_for_executor(unique_string, executor_type, executor) - unique_string = _adjust_string_with_rls(unique_string, datasources, executor) + unique_string = _adjust_string_with_rls( + unique_string, dashboard.datasources, executor + ) return md5_sha_from_str(unique_string) -def get_chart_digest(chart: Slice) -> str: +def get_chart_digest(chart: Slice) -> str | None: config = current_app.config - datasource = chart.datasource - executor_type, executor = get_executor( - executor_types=config["THUMBNAIL_EXECUTE_AS"], - model=chart, - current_user=get_current_user(), - ) + try: + executor_type, executor = get_executor( + executors=config["THUMBNAIL_EXECUTORS"], + model=chart, + current_user=get_current_user(), + ) + except ExecutorNotFoundError: + return None if func := config["THUMBNAIL_CHART_DIGEST_FUNC"]: return func(chart, executor_type, executor) unique_string = f"{chart.params or ''}.{executor}" unique_string = _adjust_string_for_executor(unique_string, executor_type, executor) - unique_string = _adjust_string_with_rls(unique_string, [datasource], executor) + unique_string = _adjust_string_with_rls(unique_string, [chart.datasource], executor) return md5_sha_from_str(unique_string) diff --git a/superset/utils/screenshots.py b/superset/utils/screenshots.py index 96c0f40d6da51..1557bc283379b 100644 --- a/superset/utils/screenshots.py +++ b/superset/utils/screenshots.py @@ -56,15 +56,18 @@ class BaseScreenshot: driver_type = current_app.config["WEBDRIVER_TYPE"] + url: str + digest: str | None + screenshot: bytes | None thumbnail_type: str = "" element: str = "" window_size: WindowSize = DEFAULT_SCREENSHOT_WINDOW_SIZE thumb_size: WindowSize = DEFAULT_SCREENSHOT_THUMBNAIL_SIZE - def __init__(self, url: str, digest: str): - self.digest: str = digest + def __init__(self, url: str, digest: str | None): + self.digest = digest self.url = url - self.screenshot: bytes | None = None + self.screenshot = None def driver(self, window_size: WindowSize | None = None) -> WebDriver: window_size = window_size or self.window_size @@ -227,7 +230,7 @@ class ChartScreenshot(BaseScreenshot): def __init__( self, url: str, - digest: str, + digest: str | None, window_size: WindowSize | None = None, thumb_size: WindowSize | None = None, ): @@ -248,7 +251,7 @@ class DashboardScreenshot(BaseScreenshot): def __init__( self, url: str, - digest: str, + digest: str | None, window_size: WindowSize | None = None, thumb_size: WindowSize | None = None, ): diff --git a/tests/integration_tests/reports/alert_tests.py b/tests/integration_tests/reports/alert_tests.py index 16ce8f3fed34b..0e1b21569362b 100644 --- a/tests/integration_tests/reports/alert_tests.py +++ b/tests/integration_tests/reports/alert_tests.py @@ -26,7 +26,7 @@ from superset.commands.report.exceptions import AlertQueryError from superset.reports.models import ReportCreationMethod, ReportScheduleType -from superset.tasks.types import ExecutorType +from superset.tasks.types import ExecutorType, FixedExecutor from superset.utils.database import get_example_database from tests.integration_tests.test_app import app @@ -34,7 +34,7 @@ @pytest.mark.parametrize( "owner_names,creator_name,config,expected_result", [ - (["gamma"], None, [ExecutorType.SELENIUM], "admin"), + (["gamma"], None, [FixedExecutor("admin")], "admin"), (["gamma"], None, [ExecutorType.OWNER], "gamma"), ( ["alpha", "gamma"], @@ -69,8 +69,8 @@ def test_execute_query_as_report_executor( from superset.commands.report.alert import AlertCommand from superset.reports.models import ReportSchedule - original_config = app.config["ALERT_REPORTS_EXECUTE_AS"] - app.config["ALERT_REPORTS_EXECUTE_AS"] = config + original_config = app.config["ALERT_REPORTS_EXECUTORS"] + app.config["ALERT_REPORTS_EXECUTORS"] = config owners = [get_user(owner_name) for owner_name in owner_names] report_schedule = ReportSchedule( created_by=get_user(creator_name) if creator_name else None, @@ -96,7 +96,7 @@ def test_execute_query_as_report_executor( command.run() assert override_user_mock.call_args[0][0].username == expected_result - app.config["ALERT_REPORTS_EXECUTE_AS"] = original_config + app.config["ALERT_REPORTS_EXECUTORS"] = original_config def test_execute_query_mutate_query_enabled( @@ -278,7 +278,7 @@ def test_get_alert_metadata_from_object( from superset.commands.report.alert import AlertCommand from superset.reports.models import ReportSchedule - app.config["ALERT_REPORTS_EXECUTE_AS"] = [ExecutorType.OWNER] + app.config["ALERT_REPORTS_EXECUTORS"] = [ExecutorType.OWNER] mock_database = mocker.MagicMock() mock_exec_id = uuid.uuid4() diff --git a/tests/integration_tests/reports/commands_tests.py b/tests/integration_tests/reports/commands_tests.py index a3f105a419536..7b22b38fc0cfc 100644 --- a/tests/integration_tests/reports/commands_tests.py +++ b/tests/integration_tests/reports/commands_tests.py @@ -773,7 +773,7 @@ def test_email_chart_report_schedule_alpha_owner( ExecuteReport Command: Test chart email report schedule with screenshot executed as the chart owner """ - config_key = "ALERT_REPORTS_EXECUTE_AS" + config_key = "ALERT_REPORTS_EXECUTORS" original_config_value = app.config[config_key] app.config[config_key] = [ExecutorType.OWNER] diff --git a/tests/integration_tests/strategy_tests.py b/tests/integration_tests/strategy_tests.py index e5901b5b82cb7..6dc99f501fee6 100644 --- a/tests/integration_tests/strategy_tests.py +++ b/tests/integration_tests/strategy_tests.py @@ -82,11 +82,15 @@ def test_top_n_dashboards_strategy(self): self.client.get(f"/superset/dashboard/{dash.id}/") strategy = TopNDashboardsStrategy(1) - result = strategy.get_payloads() + result = strategy.get_tasks() expected = [ - {"chart_id": chart.id, "dashboard_id": dash.id} for chart in dash.slices + { + "payload": {"chart_id": chart.id, "dashboard_id": dash.id}, + "username": "admin", + } + for chart in dash.slices ] - self.assertCountEqual(result, expected) # noqa: PT009 + assert len(result) == len(expected) def reset_tag(self, tag): """Remove associated object from tag, used to reset tests""" @@ -104,34 +108,30 @@ def test_dashboard_tags_strategy(self): self.reset_tag(tag1) strategy = DashboardTagsStrategy(["tag1"]) - result = strategy.get_payloads() - expected = [] - assert result == expected + assert strategy.get_tasks() == [] # tag dashboard 'births' with `tag1` tag1 = get_tag("tag1", db.session, TagType.custom) dash = self.get_dash_by_slug("births") - tag1_urls = [{"chart_id": chart.id} for chart in dash.slices] + tag1_payloads = [{"chart_id": chart.id} for chart in dash.slices] tagged_object = TaggedObject( tag_id=tag1.id, object_id=dash.id, object_type=ObjectType.dashboard ) db.session.add(tagged_object) db.session.commit() - self.assertCountEqual(strategy.get_payloads(), tag1_urls) # noqa: PT009 + assert len(strategy.get_tasks()) == len(tag1_payloads) strategy = DashboardTagsStrategy(["tag2"]) tag2 = get_tag("tag2", db.session, TagType.custom) self.reset_tag(tag2) - result = strategy.get_payloads() - expected = [] - assert result == expected + assert strategy.get_tasks() == [] # tag first slice dash = self.get_dash_by_slug("unicode-test") chart = dash.slices[0] - tag2_urls = [{"chart_id": chart.id}] + tag2_payloads = [{"chart_id": chart.id}] object_id = chart.id tagged_object = TaggedObject( tag_id=tag2.id, object_id=object_id, object_type=ObjectType.chart @@ -139,11 +139,8 @@ def test_dashboard_tags_strategy(self): db.session.add(tagged_object) db.session.commit() - result = strategy.get_payloads() - self.assertCountEqual(result, tag2_urls) # noqa: PT009 + assert len(strategy.get_tasks()) == len(tag2_payloads) strategy = DashboardTagsStrategy(["tag1", "tag2"]) - result = strategy.get_payloads() - expected = tag1_urls + tag2_urls - self.assertCountEqual(result, expected) # noqa: PT009 + assert len(strategy.get_tasks()) == len(tag1_payloads + tag2_payloads) diff --git a/tests/integration_tests/thumbnails_tests.py b/tests/integration_tests/thumbnails_tests.py index ccb9a9c734c13..e808858fb306a 100644 --- a/tests/integration_tests/thumbnails_tests.py +++ b/tests/integration_tests/thumbnails_tests.py @@ -30,7 +30,7 @@ from superset.extensions import machine_auth_provider_factory from superset.models.dashboard import Dashboard from superset.models.slice import Slice -from superset.tasks.types import ExecutorType +from superset.tasks.types import ExecutorType, FixedExecutor from superset.utils import json from superset.utils.screenshots import ChartScreenshot, DashboardScreenshot from superset.utils.urls import get_url_path @@ -53,8 +53,8 @@ def create_app(self): return app def url_open_auth(self, username: str, url: str): - admin_user = security_manager.find_user(username=username) - cookies = machine_auth_provider_factory.instance.get_auth_cookies(admin_user) + user = security_manager.find_user(username=username) + cookies = machine_auth_provider_factory.instance.get_auth_cookies(user) opener = urllib.request.build_opener() opener.addheaders.append(("Cookie", f"session={cookies['session']}")) return opener.open(f"{self.get_server_url()}/{url}") @@ -70,7 +70,7 @@ def test_get_async_dashboard_screenshot(self): thumbnail_url = resp["result"][0]["thumbnail_url"] response = self.url_open_auth( - "admin", + ADMIN_USERNAME, thumbnail_url, ) assert response.getcode() == 202 @@ -84,9 +84,7 @@ def test_not_call_find_unexpected_errors_if_feature_disabled( self, mock_find_unexpected_errors, mock_firefox, mock_webdriver_wait ): webdriver_proxy = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.dashboard", dashboard_id_or_slug=1) webdriver_proxy.get_screenshot(url, "grid-container", user=user) @@ -100,9 +98,7 @@ def test_call_find_unexpected_errors_if_feature_enabled( ): app.config["SCREENSHOT_REPLACE_UNEXPECTED_ERRORS"] = True webdriver_proxy = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.dashboard", dashboard_id_or_slug=1) webdriver_proxy.get_screenshot(url, "grid-container", user=user) @@ -149,9 +145,7 @@ def test_screenshot_selenium_headstart( self, mock_sleep, mock_webdriver, mock_webdriver_wait ): webdriver = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.slice", slice_id=1, standalone="true") app.config["SCREENSHOT_SELENIUM_HEADSTART"] = 5 webdriver.get_screenshot(url, "chart-container", user=user) @@ -162,9 +156,7 @@ def test_screenshot_selenium_headstart( def test_screenshot_selenium_locate_wait(self, mock_webdriver, mock_webdriver_wait): app.config["SCREENSHOT_LOCATE_WAIT"] = 15 webdriver = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.slice", slice_id=1, standalone="true") webdriver.get_screenshot(url, "chart-container", user=user) assert mock_webdriver_wait.call_args_list[0] == call(ANY, 15) @@ -174,9 +166,7 @@ def test_screenshot_selenium_locate_wait(self, mock_webdriver, mock_webdriver_wa def test_screenshot_selenium_load_wait(self, mock_webdriver, mock_webdriver_wait): app.config["SCREENSHOT_LOAD_WAIT"] = 15 webdriver = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.slice", slice_id=1, standalone="true") webdriver.get_screenshot(url, "chart-container", user=user) assert mock_webdriver_wait.call_args_list[2] == call(ANY, 15) @@ -188,9 +178,7 @@ def test_screenshot_selenium_animation_wait( self, mock_sleep, mock_webdriver, mock_webdriver_wait ): webdriver = WebDriverSelenium("firefox") - user = security_manager.get_user_by_username( - app.config["THUMBNAIL_SELENIUM_USER"] - ) + user = security_manager.get_user_by_username(ADMIN_USERNAME) url = get_url_path("Superset.slice", slice_id=1, standalone="true") app.config["SCREENSHOT_SELENIUM_ANIMATION_WAIT"] = 4 webdriver.get_screenshot(url, "chart-container", user=user) @@ -232,7 +220,7 @@ def test_chart_thumbnail_disabled(self): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @with_feature_flags(THUMBNAILS=True) - def test_get_async_dashboard_screenshot_as_selenium(self): + def test_get_async_dashboard_screenshot_as_fixed_user(self): """ Thumbnails: Simple get async dashboard screenshot as selenium user """ @@ -241,7 +229,7 @@ def test_get_async_dashboard_screenshot_as_selenium(self): patch.dict( "superset.thumbnails.digest.current_app.config", { - "THUMBNAIL_EXECUTE_AS": [ExecutorType.SELENIUM], + "THUMBNAIL_EXECUTORS": [FixedExecutor(ADMIN_USERNAME)], }, ), patch( @@ -251,8 +239,8 @@ def test_get_async_dashboard_screenshot_as_selenium(self): mock_adjust_string.return_value = self.digest_return_value _, thumbnail_url = self._get_id_and_thumbnail_url(DASHBOARD_URL) assert self.digest_hash in thumbnail_url - assert mock_adjust_string.call_args[0][1] == ExecutorType.SELENIUM - assert mock_adjust_string.call_args[0][2] == "admin" + assert mock_adjust_string.call_args[0][1] == ExecutorType.FIXED_USER + assert mock_adjust_string.call_args[0][2] == ADMIN_USERNAME rv = self.client.get(thumbnail_url) assert rv.status_code == 202 @@ -269,7 +257,7 @@ def test_get_async_dashboard_screenshot_as_current_user(self): patch.dict( "superset.thumbnails.digest.current_app.config", { - "THUMBNAIL_EXECUTE_AS": [ExecutorType.CURRENT_USER], + "THUMBNAIL_EXECUTORS": [ExecutorType.CURRENT_USER], }, ), patch( @@ -310,7 +298,7 @@ def test_get_async_dashboard_not_allowed(self): @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices") @with_feature_flags(THUMBNAILS=True) - def test_get_async_chart_screenshot_as_selenium(self): + def test_get_async_chart_screenshot_as_fixed_user(self): """ Thumbnails: Simple get async chart screenshot as selenium user """ @@ -319,7 +307,7 @@ def test_get_async_chart_screenshot_as_selenium(self): patch.dict( "superset.thumbnails.digest.current_app.config", { - "THUMBNAIL_EXECUTE_AS": [ExecutorType.SELENIUM], + "THUMBNAIL_EXECUTORS": [FixedExecutor(ADMIN_USERNAME)], }, ), patch( @@ -329,8 +317,8 @@ def test_get_async_chart_screenshot_as_selenium(self): mock_adjust_string.return_value = self.digest_return_value _, thumbnail_url = self._get_id_and_thumbnail_url(CHART_URL) assert self.digest_hash in thumbnail_url - assert mock_adjust_string.call_args[0][1] == ExecutorType.SELENIUM - assert mock_adjust_string.call_args[0][2] == "admin" + assert mock_adjust_string.call_args[0][1] == ExecutorType.FIXED_USER + assert mock_adjust_string.call_args[0][2] == ADMIN_USERNAME rv = self.client.get(thumbnail_url) assert rv.status_code == 202 @@ -347,7 +335,7 @@ def test_get_async_chart_screenshot_as_current_user(self): patch.dict( "superset.thumbnails.digest.current_app.config", { - "THUMBNAIL_EXECUTE_AS": [ExecutorType.CURRENT_USER], + "THUMBNAIL_EXECUTORS": [ExecutorType.CURRENT_USER], }, ), patch( diff --git a/tests/unit_tests/tasks/test_utils.py b/tests/unit_tests/tasks/test_utils.py index 2d85a1c05678c..d4b24c66661b6 100644 --- a/tests/unit_tests/tasks/test_utils.py +++ b/tests/unit_tests/tasks/test_utils.py @@ -23,11 +23,11 @@ import pytest from flask_appbuilder.security.sqla.models import User -from superset.tasks.exceptions import ExecutorNotFoundError -from superset.tasks.types import ExecutorType +from superset.tasks.exceptions import ExecutorNotFoundError, InvalidExecutorError +from superset.tasks.types import Executor, ExecutorType, FixedExecutor -SELENIUM_USER_ID = 1234 -SELENIUM_USERNAME = "admin" +FIXED_USER_ID = 1234 +FIXED_USERNAME = "admin" def _get_users( @@ -54,18 +54,18 @@ class ModelType(int, Enum): @pytest.mark.parametrize( - "model_type,executor_types,model_config,current_user,expected_result", + "model_type,executors,model_config,current_user,expected_result", [ ( ModelType.REPORT_SCHEDULE, - [ExecutorType.SELENIUM], + [FixedExecutor(FIXED_USERNAME)], ModelConfig( owners=[1, 2], creator=3, modifier=4, ), None, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ( ModelType.REPORT_SCHEDULE, @@ -75,11 +75,11 @@ class ModelType(int, Enum): ExecutorType.OWNER, ExecutorType.MODIFIER, ExecutorType.MODIFIER_OWNER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[]), None, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ( ModelType.REPORT_SCHEDULE, @@ -89,7 +89,7 @@ class ModelType(int, Enum): ExecutorType.OWNER, ExecutorType.MODIFIER, ExecutorType.MODIFIER_OWNER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[], modifier=1), None, @@ -103,7 +103,7 @@ class ModelType(int, Enum): ExecutorType.OWNER, ExecutorType.MODIFIER, ExecutorType.MODIFIER_OWNER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[2], modifier=1), None, @@ -117,7 +117,7 @@ class ModelType(int, Enum): ExecutorType.OWNER, ExecutorType.MODIFIER, ExecutorType.MODIFIER_OWNER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[2], creator=3, modifier=1), None, @@ -198,11 +198,11 @@ class ModelType(int, Enum): ( ModelType.DASHBOARD, [ - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[1], creator=2, modifier=3), 4, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ( ModelType.DASHBOARD, @@ -219,11 +219,11 @@ class ModelType(int, Enum): ExecutorType.CREATOR_OWNER, ExecutorType.MODIFIER_OWNER, ExecutorType.CURRENT_USER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[1], creator=2, modifier=3), None, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ( ModelType.CHART, @@ -237,11 +237,11 @@ class ModelType(int, Enum): ( ModelType.CHART, [ - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[1], creator=2, modifier=3), 4, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ( ModelType.CHART, @@ -252,26 +252,35 @@ class ModelType(int, Enum): None, ExecutorNotFoundError(), ), + ( + ModelType.CHART, + [ + ExecutorType.FIXED_USER, + ], + ModelConfig(owners=[]), + None, + InvalidExecutorError(), + ), ( ModelType.CHART, [ ExecutorType.CREATOR_OWNER, ExecutorType.MODIFIER_OWNER, ExecutorType.CURRENT_USER, - ExecutorType.SELENIUM, + FixedExecutor(FIXED_USERNAME), ], ModelConfig(owners=[1], creator=2, modifier=3), None, - (ExecutorType.SELENIUM, SELENIUM_USER_ID), + (ExecutorType.FIXED_USER, FIXED_USER_ID), ), ], ) def test_get_executor( model_type: ModelType, - executor_types: list[ExecutorType], + executors: list[Executor], model_config: ModelConfig, current_user: Optional[int], - expected_result: tuple[int, ExecutorNotFoundError], + expected_result: tuple[ExecutorType, int] | Exception, ) -> None: from superset.models.dashboard import Dashboard from superset.models.slice import Slice @@ -308,14 +317,14 @@ def test_get_executor( cm = nullcontext() expected_executor_type = expected_result[0] expected_executor = ( - SELENIUM_USERNAME - if expected_executor_type == ExecutorType.SELENIUM + FIXED_USERNAME + if expected_executor_type == ExecutorType.FIXED_USER else str(expected_result[1]) ) with cm: executor_type, executor = get_executor( - executor_types=executor_types, + executors=executors, model=obj, current_user=str(current_user) if current_user else None, ) diff --git a/tests/unit_tests/thumbnails/test_digest.py b/tests/unit_tests/thumbnails/test_digest.py index b08b896918fbc..aa5d8d08aea12 100644 --- a/tests/unit_tests/thumbnails/test_digest.py +++ b/tests/unit_tests/thumbnails/test_digest.py @@ -24,8 +24,8 @@ from flask_appbuilder.security.sqla.models import User from superset.connectors.sqla.models import BaseDatasource, SqlaTable -from superset.tasks.exceptions import ExecutorNotFoundError -from superset.tasks.types import ExecutorType +from superset.tasks.exceptions import InvalidExecutorError +from superset.tasks.types import Executor, ExecutorType, FixedExecutor from superset.utils.core import DatasourceType, override_user if TYPE_CHECKING: @@ -81,7 +81,7 @@ def prepare_datasource_mock( [ ( None, - [ExecutorType.SELENIUM], + [FixedExecutor("admin")], False, False, [], @@ -214,13 +214,21 @@ def prepare_datasource_mock( False, False, [], - ExecutorNotFoundError(), + None, + ), + ( + None, + [ExecutorType.FIXED_USER], + False, + False, + [], + InvalidExecutorError(), ), ], ) def test_dashboard_digest( dashboard_overrides: dict[str, Any] | None, - execute_as: list[ExecutorType], + execute_as: list[Executor], has_current_user: bool, use_custom_digest: bool, rls_datasources: list[dict[str, Any]], @@ -255,7 +263,7 @@ def test_dashboard_digest( patch.dict( app.config, { - "THUMBNAIL_EXECUTE_AS": execute_as, + "THUMBNAIL_EXECUTORS": execute_as, "THUMBNAIL_DASHBOARD_DIGEST_FUNC": func, }, ), @@ -282,7 +290,7 @@ def test_dashboard_digest( [ ( None, - [ExecutorType.SELENIUM], + [FixedExecutor("admin")], False, False, None, @@ -345,13 +353,21 @@ def test_dashboard_digest( False, False, None, - ExecutorNotFoundError(), + None, + ), + ( + None, + [ExecutorType.FIXED_USER], + False, + False, + None, + InvalidExecutorError(), ), ], ) def test_chart_digest( chart_overrides: dict[str, Any] | None, - execute_as: list[ExecutorType], + execute_as: list[Executor], has_current_user: bool, use_custom_digest: bool, rls_datasource: dict[str, Any] | None, @@ -383,7 +399,7 @@ def test_chart_digest( patch.dict( app.config, { - "THUMBNAIL_EXECUTE_AS": execute_as, + "THUMBNAIL_EXECUTORS": execute_as, "THUMBNAIL_CHART_DIGEST_FUNC": func, }, ),