diff --git a/UPDATING.md b/UPDATING.md index 663df68cd..2a9e8992d 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 293ed3f71..5ff1ef4b8 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 6d761c56b..c0eadca95 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 8d897d5bd..cba0a0897 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 0ab150e52..42852fef4 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 faecc0bc4..38e5e0fe5 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 d713c4581..458f78fd3 100644 --- a/superset/commands/report/alert.py +++ b/superset/commands/report/alert.py @@ -170,7 +170,7 @@ class AlertCommand(BaseCommand): ) 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 54a2890a9..9e6925865 100644 --- a/superset/commands/report/execute.py +++ b/superset/commands/report/execute.py @@ -295,7 +295,7 @@ class BaseReportState: :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 @@ class BaseReportState: 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 @@ class BaseReportState: """ 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 @@ class AsyncExecuteReportScheduleCommand(BaseCommand): 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 ae944f3ca..89fbdf35a 100644 --- a/superset/config.py +++ b/superset/config.py @@ -689,6 +689,15 @@ THEME_OVERRIDES: dict[str, Any] = {} # 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 @@ EXTRA_SEQUENTIAL_COLOR_SCHEMES: list[dict[str, Any]] = [] # 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 @@ ALERT_REPORTS_WORKING_TIME_OUT_KILL = True # # 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 1295f0b20..5b18e856c 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 3af3a63ab..5b8675a6a 100644 --- a/superset/models/dashboard.py +++ b/superset/models/dashboard.py @@ -225,16 +225,19 @@ class Dashboard(AuditMixinNullable, ImportExportMixin, Model): 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 1e6daa832..8795d186e 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -247,16 +247,19 @@ class Slice( # pylint: disable=too-many-public-methods } @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 b1eaff58b..1c8012ff8 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 = get_task_logger(__name__) 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 @@ class TopNDashboardsStrategy(Strategy): # pylint: disable=too-few-public-method 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 @@ class TopNDashboardsStrategy(Strategy): # pylint: disable=too-few-public-method ) return [ - get_payload(chart, dashboard) + get_task(chart, dashboard) for dashboard in dashboards for chart in dashboard.slices ] @@ -167,8 +190,8 @@ class DashboardTagsStrategy(Strategy): # pylint: disable=too-few-public-methods 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 @@ class DashboardTagsStrategy(Strategy): # pylint: disable=too-few-public-methods ) 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 @@ class DashboardTagsStrategy(Strategy): # pylint: disable=too-few-public-methods 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 669866175..19a97ea65 100644 --- a/superset/tasks/exceptions.py +++ b/superset/tasks/exceptions.py @@ -22,3 +22,7 @@ from superset.exceptions import SupersetException 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 dd9b5065d..3b0b47dbb 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 84a3e7b01..8f2f76b40 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 4815b7034..845ea2b5f 100644 --- a/superset/tasks/utils.py +++ b/superset/tasks/utils.py @@ -23,10 +23,10 @@ from typing import Optional, TYPE_CHECKING 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 @@ logger.setLevel(logging.INFO) # 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 c10e4330c..446d06b20 100644 --- a/superset/thumbnails/digest.py +++ b/superset/thumbnails/digest.py @@ -23,6 +23,7 @@ from typing import TYPE_CHECKING 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 96c0f40d6..1557bc283 100644 --- a/superset/utils/screenshots.py +++ b/superset/utils/screenshots.py @@ -56,15 +56,18 @@ if TYPE_CHECKING: 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 16ce8f3fe..0e1b21569 100644 --- a/tests/integration_tests/reports/alert_tests.py +++ b/tests/integration_tests/reports/alert_tests.py @@ -26,7 +26,7 @@ from pytest_mock import MockerFixture 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 @@ from tests.integration_tests.test_app import app @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 a3f105a41..7b22b38fc 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 e5901b5b8..6dc99f501 100644 --- a/tests/integration_tests/strategy_tests.py +++ b/tests/integration_tests/strategy_tests.py @@ -82,11 +82,15 @@ class TestCacheWarmUp(SupersetTestCase): 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 @@ class TestCacheWarmUp(SupersetTestCase): 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 @@ class TestCacheWarmUp(SupersetTestCase): 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 ccb9a9c73..e808858fb 100644 --- a/tests/integration_tests/thumbnails_tests.py +++ b/tests/integration_tests/thumbnails_tests.py @@ -30,7 +30,7 @@ from superset import db, is_feature_enabled, security_manager 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 @@ class TestThumbnailsSeleniumLive(LiveServerTestCase): 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 @@ class TestThumbnailsSeleniumLive(LiveServerTestCase): 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 @@ class TestWebDriverScreenshotErrorDetector(SupersetTestCase): 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 @@ class TestWebDriverScreenshotErrorDetector(SupersetTestCase): ): 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 @@ class TestWebDriverSelenium(SupersetTestCase): 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 @@ class TestWebDriverSelenium(SupersetTestCase): 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 @@ class TestWebDriverSelenium(SupersetTestCase): 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 @@ class TestWebDriverSelenium(SupersetTestCase): 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 @@ class TestThumbnails(SupersetTestCase): @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 @@ class TestThumbnails(SupersetTestCase): patch.dict( "superset.thumbnails.digest.current_app.config", { - "THUMBNAIL_EXECUTE_AS": [ExecutorType.SELENIUM], + "THUMBNAIL_EXECUTORS": [FixedExecutor(ADMIN_USERNAME)], }, ), patch( @@ -251,8 +239,8 @@ class TestThumbnails(SupersetTestCase): 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 @@ class TestThumbnails(SupersetTestCase): patch.dict( "superset.thumbnails.digest.current_app.config", { - "THUMBNAIL_EXECUTE_AS": [ExecutorType.CURRENT_USER], + "THUMBNAIL_EXECUTORS": [ExecutorType.CURRENT_USER], }, ), patch( @@ -310,7 +298,7 @@ class TestThumbnails(SupersetTestCase): @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 @@ class TestThumbnails(SupersetTestCase): patch.dict( "superset.thumbnails.digest.current_app.config", { - "THUMBNAIL_EXECUTE_AS": [ExecutorType.SELENIUM], + "THUMBNAIL_EXECUTORS": [FixedExecutor(ADMIN_USERNAME)], }, ), patch( @@ -329,8 +317,8 @@ class TestThumbnails(SupersetTestCase): 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 @@ class TestThumbnails(SupersetTestCase): 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 2d85a1c05..d4b24c666 100644 --- a/tests/unit_tests/tasks/test_utils.py +++ b/tests/unit_tests/tasks/test_utils.py @@ -23,11 +23,11 @@ from typing import Any, Optional, Union 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 b08b89691..aa5d8d08a 100644 --- a/tests/unit_tests/thumbnails/test_digest.py +++ b/tests/unit_tests/thumbnails/test_digest.py @@ -24,8 +24,8 @@ import pytest 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, }, ),