From c32240778bbb1d74f5f05b698378623286d29ac8 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Fri, 20 Jan 2023 09:42:54 +0200 Subject: [PATCH 1/5] chore(thumbnails): change default executor --- docs/docs/installation/cache.mdx | 9 ++++++--- superset/config.py | 15 ++++++--------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/docs/docs/installation/cache.mdx b/docs/docs/installation/cache.mdx index 58b4bcb2b0b74..15450bae046ad 100644 --- a/docs/docs/installation/cache.mdx +++ b/docs/docs/installation/cache.mdx @@ -87,13 +87,16 @@ FEATURE_FLAGS = { } ``` -By default thumbnails are rendered using the `THUMBNAIL_SELENIUM_USER` user account. To render thumbnails as the -logged in user (e.g. in environments that are using user impersonation), use the following configuration: +By default thumbnails are rendered per user. To render thumbnails as a fixed user (`admin` in this example), use the following configuration: ```python -THUMBNAIL_EXECUTE_AS = [ExecutorType.CURRENT_USER] +from superset.tasks.types import ExecutorType + +THUMBNAIL_SELENIUM_USER = "admin" +THUMBNAIL_EXECUTE_AS = [ExecutorType.SELENIUM] ``` + For this feature you will need a cache system and celery workers. All thumbnails are stored on cache and are processed asynchronously by the workers. diff --git a/superset/config.py b/superset/config.py index 42dbeda852dd9..4e5ebf2791c98 100644 --- a/superset/config.py +++ b/superset/config.py @@ -611,16 +611,13 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # --------------------------------------------------- # Thumbnail config (behind feature flag) # --------------------------------------------------- -# When executing Alerts & Reports or Thumbnails as the Selenium user, this defines -# the username of the account used to render the queries and dashboards/charts +# By default, thumbnails are rendered per user. Similar to Alerts & Reports, thumbnails +# can be configured to be rendered as a fixed user. See +# `superset.tasks.types.ExecutorType` for a full list of executor options. +# To use a fixed user account, use the following configuration: +# THUMBNAIL_EXECUTE_AS = [ExecutorType.SELENIUM] THUMBNAIL_SELENIUM_USER: Optional[str] = "admin" - -# To be able to have different thumbnails for different users, use these configs to -# define which user to execute the thumbnails and potentially custom functions for -# calculating thumbnail digests. To have unique thumbnails for all users, use the -# following config: -# THUMBNAIL_EXECUTE_AS = [ExecutorType.CURRENT_USER] -THUMBNAIL_EXECUTE_AS = [ExecutorType.SELENIUM] +THUMBNAIL_EXECUTE_AS = [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 From e190ecb05d90d63011ed827056ed21adb263b4bf Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Fri, 20 Jan 2023 09:47:23 +0200 Subject: [PATCH 2/5] add updating entry --- UPDATING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/UPDATING.md b/UPDATING.md index ee489ec5cd33e..c2a94ff8c45bc 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -34,6 +34,7 @@ assists people when migrating to a new version. ### Breaking Changes +- [22801](https://github.com/apache/superset/pull/22801): The Thumbnails feature has been changed to execute as the currently logged in user by default. To continue using the selenium user, please add the following to your `superset_config.py`: `THUMBNAILS_EXECUTE_AS = ["selenium"]` - [22328](https://github.com/apache/superset/pull/22328): For deployments that have enabled the "THUMBNAILS" feature flag, the function that calculates dashboard digests has been updated to consider additional properties to more accurately identify changes in the dashboard metadata. This change will invalidate all currently cached dashboard thumbnails. - [21765](https://github.com/apache/superset/pull/21765): For deployments that have enabled the "ALERT_REPORTS" feature flag, Gamma users will no longer have read and write access to Alerts & Reports by default. To give Gamma users the ability to schedule reports from the Dashboard and Explore view like before, create an additional role with "can read on ReportSchedule" and "can write on ReportSchedule" permissions. To further give Gamma users access to the "Alerts & Reports" menu and CRUD view, add "menu access on Manage" and "menu access on Alerts & Report" permissions to the role. - [22325](https://github.com/apache/superset/pull/22325): "RLS_FORM_QUERY_REL_FIELDS" is replaced by "RLS_BASE_RELATED_FIELD_FILTERS" feature flag.Its value format stays same. From 23b8c08832f6e6ac7a12ef268d9b1112c9d11650 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Fri, 20 Jan 2023 10:24:57 +0200 Subject: [PATCH 3/5] fall back to selenium user --- UPDATING.md | 2 +- docs/docs/installation/cache.mdx | 3 ++- superset/config.py | 9 +++++---- tests/integration_tests/thumbnails_tests.py | 14 ++++++++++++-- 4 files changed, 20 insertions(+), 8 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index c2a94ff8c45bc..2ebe7fb111dd6 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -34,7 +34,7 @@ assists people when migrating to a new version. ### Breaking Changes -- [22801](https://github.com/apache/superset/pull/22801): The Thumbnails feature has been changed to execute as the currently logged in user by default. To continue using the selenium user, please add the following to your `superset_config.py`: `THUMBNAILS_EXECUTE_AS = ["selenium"]` +- [22801](https://github.com/apache/superset/pull/22801): The Thumbnails feature has been changed to execute as the currently logged in user by default, falling back to the selenium user for anonymous users. To continue always using the selenium user, please add the following to your `superset_config.py`: `THUMBNAILS_EXECUTE_AS = ["selenium"]` - [22328](https://github.com/apache/superset/pull/22328): For deployments that have enabled the "THUMBNAILS" feature flag, the function that calculates dashboard digests has been updated to consider additional properties to more accurately identify changes in the dashboard metadata. This change will invalidate all currently cached dashboard thumbnails. - [21765](https://github.com/apache/superset/pull/21765): For deployments that have enabled the "ALERT_REPORTS" feature flag, Gamma users will no longer have read and write access to Alerts & Reports by default. To give Gamma users the ability to schedule reports from the Dashboard and Explore view like before, create an additional role with "can read on ReportSchedule" and "can write on ReportSchedule" permissions. To further give Gamma users access to the "Alerts & Reports" menu and CRUD view, add "menu access on Manage" and "menu access on Alerts & Report" permissions to the role. - [22325](https://github.com/apache/superset/pull/22325): "RLS_FORM_QUERY_REL_FIELDS" is replaced by "RLS_BASE_RELATED_FIELD_FILTERS" feature flag.Its value format stays same. diff --git a/docs/docs/installation/cache.mdx b/docs/docs/installation/cache.mdx index 15450bae046ad..5b2d908961dcd 100644 --- a/docs/docs/installation/cache.mdx +++ b/docs/docs/installation/cache.mdx @@ -87,7 +87,8 @@ FEATURE_FLAGS = { } ``` -By default thumbnails are rendered per user. To render thumbnails as a fixed user (`admin` in this example), use the following configuration: +By default thumbnails are rendered per user, and will fall back to the Selenium user for anonymous users. +To always render thumbnails as a fixed user (`admin` in this example), use the following configuration: ```python from superset.tasks.types import ExecutorType diff --git a/superset/config.py b/superset/config.py index 4e5ebf2791c98..c501446fcb906 100644 --- a/superset/config.py +++ b/superset/config.py @@ -611,13 +611,14 @@ def _try_json_readsha(filepath: str, length: int) -> Optional[str]: # --------------------------------------------------- # Thumbnail config (behind feature flag) # --------------------------------------------------- -# By default, thumbnails are rendered per user. Similar to Alerts & Reports, thumbnails -# can be configured to be rendered as a fixed user. See +# By default, thumbnails are rendered per user, and will fall back to the Selenium +# 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 use a fixed user account, use the following configuration: +# To always use a fixed user account, use the following configuration: # THUMBNAIL_EXECUTE_AS = [ExecutorType.SELENIUM] THUMBNAIL_SELENIUM_USER: Optional[str] = "admin" -THUMBNAIL_EXECUTE_AS = [ExecutorType.CURRENT_USER] +THUMBNAIL_EXECUTE_AS = [ExecutorType.CURRENT_USER, ExecutorType.SELENIUM] # By default, thumbnail digests are calculated based on various parameters in the # chart/dashboard metadata, and in the case of user-specific thumbnails, the diff --git a/tests/integration_tests/thumbnails_tests.py b/tests/integration_tests/thumbnails_tests.py index efa0d73cb49f0..5cb45a7637a5c 100644 --- a/tests/integration_tests/thumbnails_tests.py +++ b/tests/integration_tests/thumbnails_tests.py @@ -239,7 +239,12 @@ def test_get_async_dashboard_screenshot_as_selenium(self): Thumbnails: Simple get async dashboard screenshot as selenium user """ self.login(username="alpha") - with patch( + with patch.dict( + "superset.thumbnails.digest.current_app.config", + { + "THUMBNAIL_EXECUTE_AS": [ExecutorType.SELENIUM], + }, + ), patch( "superset.thumbnails.digest._adjust_string_for_executor" ) as mock_adjust_string: mock_adjust_string.return_value = self.digest_return_value @@ -306,7 +311,12 @@ def test_get_async_chart_screenshot_as_selenium(self): Thumbnails: Simple get async chart screenshot as selenium user """ self.login(username="alpha") - with patch( + with patch.dict( + "superset.thumbnails.digest.current_app.config", + { + "THUMBNAIL_EXECUTE_AS": [ExecutorType.SELENIUM], + }, + ), patch( "superset.thumbnails.digest._adjust_string_for_executor" ) as mock_adjust_string: mock_adjust_string.return_value = self.digest_return_value From f37afc70c7e5365c9d16e786fe267bab5ddb4a69 Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Sun, 22 Jan 2023 13:56:44 +0200 Subject: [PATCH 4/5] fix test --- .../integration_tests/dashboards/api_tests.py | 63 ++++++++++--------- 1 file changed, 32 insertions(+), 31 deletions(-) diff --git a/tests/integration_tests/dashboards/api_tests.py b/tests/integration_tests/dashboards/api_tests.py index 7288889bf1157..73b6a7146a585 100644 --- a/tests/integration_tests/dashboards/api_tests.py +++ b/tests/integration_tests/dashboards/api_tests.py @@ -36,7 +36,7 @@ from superset.models.core import FavStar, FavStarClassName from superset.reports.models import ReportSchedule, ReportScheduleType from superset.models.slice import Slice -from superset.utils.core import backend +from superset.utils.core import backend, override_user from superset.views.base import generate_download_headers from tests.integration_tests.base_api_tests import ApiOwnersTestCaseMixin @@ -404,39 +404,40 @@ def test_get_dashboard(self): uri = f"api/v1/dashboard/{dashboard.id}" rv = self.get_assert_metric(uri, "get") self.assertEqual(rv.status_code, 200) - expected_result = { - "certified_by": None, - "certification_details": None, - "changed_by": None, - "changed_by_name": "", - "changed_by_url": "", - "charts": [], - "created_by": { - "id": 1, - "first_name": "admin", - "last_name": "user", - }, - "id": dashboard.id, - "css": "", - "dashboard_title": "title", - "datasources": [], - "json_metadata": "", - "owners": [ - { + with override_user(admin): + expected_result = { + "certified_by": None, + "certification_details": None, + "changed_by": None, + "changed_by_name": "", + "changed_by_url": "", + "charts": [], + "created_by": { "id": 1, - "username": "admin", "first_name": "admin", "last_name": "user", - } - ], - "roles": [], - "position_json": "", - "published": False, - "url": "/superset/dashboard/slug1/", - "slug": "slug1", - "thumbnail_url": dashboard.thumbnail_url, - "is_managed_externally": False, - } + }, + "id": dashboard.id, + "css": "", + "dashboard_title": "title", + "datasources": [], + "json_metadata": "", + "owners": [ + { + "id": 1, + "username": "admin", + "first_name": "admin", + "last_name": "user", + } + ], + "roles": [], + "position_json": "", + "published": False, + "url": "/superset/dashboard/slug1/", + "slug": "slug1", + "thumbnail_url": dashboard.thumbnail_url, + "is_managed_externally": False, + } data = json.loads(rv.data.decode("utf-8")) self.assertIn("changed_on", data["result"]) self.assertIn("changed_on_delta_humanized", data["result"]) From e4539ac3aae1145a41ac8af68f5b51c1a60dbd7d Mon Sep 17 00:00:00 2001 From: Ville Brofeldt Date: Sun, 22 Jan 2023 20:43:18 +0200 Subject: [PATCH 5/5] remove redunant digest check --- .../reports/commands/execute_dashboard_report_tests.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/tests/integration_tests/reports/commands/execute_dashboard_report_tests.py b/tests/integration_tests/reports/commands/execute_dashboard_report_tests.py index 0027738fd9807..fe20365765339 100644 --- a/tests/integration_tests/reports/commands/execute_dashboard_report_tests.py +++ b/tests/integration_tests/reports/commands/execute_dashboard_report_tests.py @@ -58,13 +58,12 @@ def test_report_for_dashboard_with_tabs( ).run() dashboard_state = report_schedule.extra.get("dashboard", {}) permalink_key = CreateDashboardPermalinkCommand( - dashboard.id, dashboard_state + str(dashboard.id), dashboard_state ).run() assert dashboard_screenshot_mock.call_count == 1 - (url, digest) = dashboard_screenshot_mock.call_args.args + url = dashboard_screenshot_mock.call_args.args[0] assert url.endswith(f"/superset/dashboard/p/{permalink_key}/") - assert digest == dashboard.digest assert send_email_smtp_mock.call_count == 1 assert len(send_email_smtp_mock.call_args.kwargs["images"]) == 1 @@ -101,9 +100,8 @@ def test_report_with_header_data( ).run() assert dashboard_screenshot_mock.call_count == 1 - (url, digest) = dashboard_screenshot_mock.call_args.args + url = dashboard_screenshot_mock.call_args.args[0] assert url.endswith(f"/superset/dashboard/p/{permalink_key}/") - assert digest == dashboard.digest assert send_email_smtp_mock.call_count == 1 header_data = send_email_smtp_mock.call_args.kwargs["header_data"] assert header_data.get("dashboard_id") == dashboard.id