Compare commits

...
Author SHA1 Message Date
Joe Li f015871ee1 fix(ci): repair nightly pre-commit failures 2026-08-20 09:51:16 -07:00
e7dccd44a7 fix(reports): prevent blank/partial report PDFs from virtualized charts (#43348)
Co-authored-by: Matt Fitzgerald <matt.fitzgerald@preset.io>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Amin Ghadersohi <amin.ghadersohi@gmail.com>
2026-08-20 09:10:30 -04:00
dependabot[bot] fdd3945dde chore(deps): bump github/codeql-action/analyze from 4.37.6 to 4.37.7 (#43361)
Signed-off-by: dependabot[bot] <support@github.com>
2026-08-20 13:58:31 +03:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> 8b67fb4d30 chore(deps-dev): bump globals from 17.10.0 to 17.11.0 in /superset-websocket (#43360)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-20 00:41:49 -07:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> b288db91f9 chore(deps): bump github/codeql-action/init from 4.37.6 to 4.37.7 (#43362)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-20 00:41:43 -07:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> 80bba12d0f chore(deps-dev): bump globals from 17.10.0 to 17.11.0 in /docs (#43363)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-20 00:41:38 -07:00
dependabot[bot]anddependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> 5d4358a149 chore(deps): bump google-auth-library from 11.0.1 to 11.0.2 in /superset-frontend (#43364)
Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
2026-08-20 00:41:25 -07:00
f9cedf84e2 fix: drop post-processing options the operation no longer accepts (#42927)
Signed-off-by: Arya Ketan <aryaketan@sharechat.co>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Evan Rusackas <evan@preset.io>
2026-08-19 18:15:15 -07:00
19 changed files with 573 additions and 59 deletions
+2 -2
View File
@@ -67,7 +67,7 @@ jobs:
# Initializes the CodeQL tools for scanning.
- name: Initialize CodeQL
uses: github/codeql-action/init@5595ccaf912efad79be6eef63a5619ff05969be3 # v4.37.6
uses: github/codeql-action/init@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7
with:
languages: ${{ matrix.language }}
# If you wish to specify custom queries, you can do so here or in a config file.
@@ -78,6 +78,6 @@ jobs:
# queries: security-extended,security-and-quality
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@5595ccaf912efad79be6eef63a5619ff05969be3 # v4.37.6
uses: github/codeql-action/analyze@ff2f1c621b7f889edc0d3c761ac2e6a3f8cdb0dd # v4.37.7
with:
category: "/language:${{matrix.language}}"
+1 -1
View File
@@ -93,7 +93,7 @@
"@typescript-eslint/parser": "^8.67.0",
"eslint": "^9.39.2",
"eslint-plugin-react": "^7.37.5",
"globals": "^17.10.0",
"globals": "^17.11.0",
"oxfmt": "^0.63.0",
"typescript": "~6.0.3",
"typescript-eslint": "^8.67.0",
+4 -4
View File
@@ -9174,10 +9174,10 @@ globals@^14.0.0:
resolved "https://registry.yarnpkg.com/globals/-/globals-14.0.0.tgz#898d7413c29babcf6bafe56fcadded858ada724e"
integrity sha512-oahGvuMGQlPw/ivIYBjVSrWAfWLBeku5tpPE2fOPLi+WHffIWbuh2tCjhyQhTBPMf5E9jDEH4FOmTYgYwbKwtQ==
globals@^17.10.0:
version "17.10.0"
resolved "https://registry.yarnpkg.com/globals/-/globals-17.10.0.tgz#f9dbd847ae99e236f98b13095e2426ac3b25a45c"
integrity sha512-V0kztuWST2k8A/VbxAY8+L+7+Rgo3fyA24IHRLrZp7HOzJjV0gHSaZUjK9lpP/IrBSNite2tZ1prhRkinRu1CA==
globals@^17.11.0:
version "17.11.0"
resolved "https://registry.yarnpkg.com/globals/-/globals-17.11.0.tgz#d643485bb30220d7751e511cf4f68c73d3870d87"
integrity sha512-Z2I8hM+PbJDXQDq3Icgpzv+mPdwr68iZUU9d5WW4FuXfDUQfkZaZuvjMv42/5crNyw154+9+VWXbYrUgDXbxNw==
globalthis@^1.0.4:
version "1.0.4"
+6 -15
View File
@@ -99,7 +99,7 @@
"geostyler-openlayers-parser": "^5.7.1",
"geostyler-style": "11.0.2",
"geostyler-wfs-parser": "^3.0.1",
"google-auth-library": "^11.0.1",
"google-auth-library": "^11.0.2",
"immer": "^11.1.16",
"interweave": "^13.1.1",
"jquery": "^4.0.0",
@@ -20612,7 +20612,7 @@
"version": "0.8.0",
"resolved": "https://registry.npmjs.org/expect-playwright/-/expect-playwright-0.8.0.tgz",
"integrity": "sha512-+kn8561vHAY+dt+0gMqqj1oY+g5xWrsuGMk4QGxotT2WS545nVqqjs37z6hrYfIuucwqthzwJfCJUEYqixyljg==",
"deprecated": "\u26a0\ufe0f The 'expect-playwright' package is deprecated. The Playwright core assertions (via @playwright/test) now cover the same functionality. Please migrate to built-in expect. See https://playwright.dev/docs/test-assertions for migration.",
"deprecated": "⚠️ The 'expect-playwright' package is deprecated. The Playwright core assertions (via @playwright/test) now cover the same functionality. Please migrate to built-in expect. See https://playwright.dev/docs/test-assertions for migration.",
"dev": true,
"license": "MIT"
},
@@ -22783,9 +22783,9 @@
"license": "MIT"
},
"node_modules/google-auth-library": {
"version": "11.0.1",
"resolved": "https://registry.npmjs.org/google-auth-library/-/google-auth-library-11.0.1.tgz",
"integrity": "sha512-ZqfaYduu9ASUaFuUk5dF9g9QvufdhhSj7jFiEnCrTQcH57sFPKYetM0iU4dcKkQk6CqC1xpSrVr5uQ9NhqjNOg==",
"version": "11.0.2",
"resolved": "https://registry.npmjs.org/google-auth-library/-/google-auth-library-11.0.2.tgz",
"integrity": "sha512-vzpgPutxrghPsnjrjpzLX2bdv8IOL719Rh0oEjGnQu8YCIbnbMuTTQ5zU9LcKvLdOPgCxBwppbvnhgW90Qna5Q==",
"license": "Apache-2.0",
"dependencies": {
"base64-js": "^1.3.0",
@@ -26023,7 +26023,7 @@
"version": "0.4.0",
"resolved": "https://registry.npmjs.org/jest-process-manager/-/jest-process-manager-0.4.0.tgz",
"integrity": "sha512-80Y6snDyb0p8GG83pDxGI/kQzwVTkCxc7ep5FPe/F6JYdvRDhwr6RzRmPSP7SEwuLhxo80lBS/NqOdUIbHIfhw==",
"deprecated": "\u26a0\ufe0f The 'jest-process-manager' package is deprecated. Please migrate to Playwright's built-in test runner (@playwright/test) which now includes full Jest-style features and parallel testing. See https://playwright.dev/docs/intro for details.",
"deprecated": "⚠️ The 'jest-process-manager' package is deprecated. Please migrate to Playwright's built-in test runner (@playwright/test) which now includes full Jest-style features and parallel testing. See https://playwright.dev/docs/intro for details.",
"dev": true,
"license": "MIT",
"dependencies": {
@@ -43073,15 +43073,6 @@
"node": ">=12"
}
},
"packages/superset-ui-core/node_modules/dompurify": {
"version": "3.4.13",
"resolved": "https://registry.npmjs.org/dompurify/-/dompurify-3.4.13.tgz",
"integrity": "sha512-2vmYIoqjze2d+kakP8S/nS5shfsl587kzwEjcGlTdiksUVgFHnFCsLYDVj/JNqJVOQZGSYBTmuycv0PodwmnMQ==",
"license": "(MPL-2.0 OR Apache-2.0)",
"optionalDependencies": {
"@types/trusted-types": "^2.0.7"
}
},
"packages/superset-ui-core/node_modules/react-ace": {
"version": "14.0.1",
"resolved": "https://registry.npmjs.org/react-ace/-/react-ace-14.0.1.tgz",
+1 -1
View File
@@ -176,7 +176,7 @@
"geostyler-openlayers-parser": "^5.7.1",
"geostyler-style": "11.0.2",
"geostyler-wfs-parser": "^3.0.1",
"google-auth-library": "^11.0.1",
"google-auth-library": "^11.0.2",
"immer": "^11.1.16",
"interweave": "^13.1.1",
"jquery": "^4.0.0",
@@ -29,7 +29,9 @@ import { ControlFormItemComponents } from './ControlForm';
* Column formatting configs.
*/
export type ColumnConfig = {
[key in SharedColumnConfigProp]?: (typeof SHARED_COLUMN_CONFIG_PROPS)[key]['value'];
[
key in SharedColumnConfigProp
]?: (typeof SHARED_COLUMN_CONFIG_PROPS)[key]['value'];
} & Record<string, StrictJsonValue>;
/**
@@ -22,10 +22,11 @@ import FixedOrMetricControl from '.';
jest.mock(
'@superset-ui/core/components/Icons/AsyncIcon',
() =>
({ fileName }: { fileName: string }) => (
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
({ fileName }: { fileName: string }) =>
(
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
);
const createProps = () => ({
@@ -38,10 +38,11 @@ import {
jest.mock(
'@superset-ui/core/components/Icons/AsyncIcon',
() =>
({ fileName }: { fileName: string }) => (
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
({ fileName }: { fileName: string }) =>
(
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
);
// eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks
@@ -29,10 +29,11 @@ import DatasetPanelWrapper from 'src/features/datasets/AddDataset/DatasetPanel';
jest.mock(
'@superset-ui/core/components/Icons/AsyncIcon',
() =>
({ fileName }: { fileName: string }) => (
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
({ fileName }: { fileName: string }) =>
(
// eslint-disable-next-line jsx-a11y/prefer-tag-over-role -- mirrors AsyncIcon's real span+role="img" shape
<span role="img" aria-label={fileName.replace('_', '-')} />
),
);
const errorMessageRegistry = getErrorMessageComponentRegistry();
+4 -4
View File
@@ -28,7 +28,7 @@
"@typescript-eslint/parser": "^8.67.0",
"eslint": "^10.8.1",
"eslint-config-prettier": "^10.1.8",
"globals": "^17.10.0",
"globals": "^17.11.0",
"oxfmt": "^0.63.0",
"tscw-config": "^1.1.2",
"typescript": "^6.0.3",
@@ -2053,9 +2053,9 @@
}
},
"node_modules/globals": {
"version": "17.10.0",
"resolved": "https://registry.npmjs.org/globals/-/globals-17.10.0.tgz",
"integrity": "sha512-V0kztuWST2k8A/VbxAY8+L+7+Rgo3fyA24IHRLrZp7HOzJjV0gHSaZUjK9lpP/IrBSNite2tZ1prhRkinRu1CA==",
"version": "17.11.0",
"resolved": "https://registry.npmjs.org/globals/-/globals-17.11.0.tgz",
"integrity": "sha512-Z2I8hM+PbJDXQDq3Icgpzv+mPdwr68iZUU9d5WW4FuXfDUQfkZaZuvjMv42/5crNyw154+9+VWXbYrUgDXbxNw==",
"dev": true,
"license": "MIT",
"engines": {
+1 -1
View File
@@ -36,7 +36,7 @@
"@typescript-eslint/parser": "^8.67.0",
"eslint": "^10.8.1",
"eslint-config-prettier": "^10.1.8",
"globals": "^17.10.0",
"globals": "^17.11.0",
"oxfmt": "^0.63.0",
"tscw-config": "^1.1.2",
"typescript": "^6.0.3",
+81 -2
View File
@@ -17,6 +17,7 @@
# pylint: disable=invalid-name
from __future__ import annotations
import inspect
import logging
from datetime import datetime
from pprint import pformat
@@ -205,8 +206,86 @@ class QueryObject: # pylint: disable=too-many-instance-attributes
def _set_post_processing(
self, post_processing: list[dict[str, Any] | None] | None
) -> None:
post_processing = post_processing or []
self.post_processing = [post_proc for post_proc in post_processing if post_proc]
self.post_processing = [
self._drop_unsupported_options(post_proc)
for post_proc in post_processing or []
if post_proc
]
@staticmethod
def _drop_unsupported_options(post_proc: dict[str, Any]) -> dict[str, Any]:
"""
Drop options that the post-processing operation no longer accepts.
A chart's ``query_context`` is written when the chart is saved and is
never rewritten afterwards, while Explore rebuilds the query from
``form_data`` at every render. A chart saved by an older version of
Superset can therefore reference an option that has since been removed
from the operation. ``exec_post_processing`` passes the stored options
as keyword arguments, so that option raises a bare ``TypeError`` on
every path that replays the stored ``query_context`` -- the chart data
endpoint, alerts and reports, thumbnails, CSV export -- while the same
chart still renders correctly in Explore.
Comparing against the signature avoids a hard-coded list of removed
option names, which would need extending at each release.
"""
operation = post_proc.get("operation")
function = (
getattr(pandas_postprocessing, operation, None)
if isinstance(operation, str)
else None
)
if function is None:
# A missing or unknown operation is left untouched, so that
# exec_post_processing reports it as InvalidPostProcessingError.
return post_proc
parameters = inspect.signature(function).parameters
if any(
parameter.kind is inspect.Parameter.VAR_KEYWORD
for parameter in parameters.values()
):
return post_proc
# `exec_post_processing` calls the operation as `operation(df, **options)`,
# so an option can only reach a parameter that a caller may fill by
# keyword. That excludes the first parameter, which receives the
# DataFrame positionally, and any positional-only or `*args` parameter.
keyword_parameters = {
name
for position, (name, parameter) in enumerate(parameters.items())
if position > 0
and parameter.kind
in (
inspect.Parameter.POSITIONAL_OR_KEYWORD,
inspect.Parameter.KEYWORD_ONLY,
)
}
options = post_proc.get("options") or {}
unsupported = {key for key in options if key not in keyword_parameters}
if not unsupported:
return post_proc
# Logged at info: a chart saved before the option was removed hits this
# on every render, so a warning would repeat for as long as the chart
# is not resaved, without anything new to report.
logger.info(
"Dropping unsupported option(s) %s of post-processing operation "
"`%s`. The chart's stored query_context predates the current "
"signature of that operation.",
sorted(unsupported),
operation,
)
return {
**post_proc,
"options": {
key: value
for key, value in options.items()
if key in keyword_parameters
},
}
def _init_series_columns(
self,
+1 -1
View File
@@ -384,7 +384,7 @@ class SemanticView(AuditMixinNullable, Model):
for dimension in dimensions
},
}
column_formats = {
column_formats: dict[str, str | None] = {
metric.name: metric.d3format for metric in metrics if metric.d3format
}
@@ -15,7 +15,7 @@
# specific language governing permissions and limitations
# under the License.
from collections.abc import Sequence
from functools import partial
from functools import partial, wraps
from typing import Any, Callable
import numpy as np
@@ -122,6 +122,10 @@ def scalar_to_sequence(val: Any) -> Sequence[str]:
def validate_column_args(*argnames: str) -> Callable[..., Any]:
def wrapper(func: Callable[..., Any]) -> Callable[..., Any]:
# `wraps` keeps `func` reachable through `__wrapped__`, so that
# `inspect.signature` reports the parameters of the decorated operation
# rather than the `(df, **options)` of this wrapper.
@wraps(func)
def wrapped(df: DataFrame, **options: Any) -> Any:
if _is_multi_index_on_columns(df):
# MultiIndex column validate first level
+60 -8
View File
@@ -159,16 +159,49 @@ CHART_ERROR_OR_EMPTY_SELECTOR = (
f"{ALERT_SELECTOR}, {EMPTY_SELECTOR}, {MISSING_CHART_SELECTOR}"
)
# Shared body for holder readiness and timeout diagnostics. A holder is ready
# only after a terminal marker appears and its loading marker disappears.
UNREADY_CHART_HOLDERS_JS_BODY = f"""
# Runtime contract with the dashboard frontend. Dispatching this window event
# forces every DashboardVirtualization row to render regardless of whether it
# intersects the headless viewport, mirroring the client-side "Download as
# Image/PDF" path (see FORCE_IN_VIEW_EVENT in
# superset-frontend/src/dashboard/constants.ts and forceLoadAllCharts in
# superset-frontend/src/utils/downloadUtils.ts). The non-tiled report capture
# takes a single full-page screenshot that includes below-the-fold holders, so
# those holders must be forced to render before the readiness wait -- otherwise
# a virtualized (or still-loading) off-screen holder is captured blank. A plain
# Event with no `detail.rowIds` means "force every row", matching the frontend's
# single-pass branch.
FORCE_ALL_CHART_HOLDERS_IN_VIEW_EVENT = "superset-force-all-in-view"
FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS = (
f"() => window.dispatchEvent(new Event('{FORCE_ALL_CHART_HOLDERS_IN_VIEW_EVENT}'))"
)
def _unready_chart_holders_js_body(*, viewport_only: bool) -> str:
"""Return the shared holder-readiness scan body.
A holder is ready only after a terminal marker appears and its loading
marker disappears. When ``viewport_only`` is True the scan skips holders
that do not intersect the current viewport: correct for the tiled path,
which scrolls every region into view before capturing it, and for
thumbnails, which only ever capture the viewport. The non-tiled *report*
capture takes a single full-page screenshot that includes below-the-fold
holders, so it must scan every mounted holder (``viewport_only=False``) --
otherwise an off-screen holder that never rendered is captured blank and
silently delivered as a Success.
"""
viewport_skip = (
"""
const r = holder.getBoundingClientRect();
if (!(r.top < window.innerHeight && r.bottom > 0)) {
continue;
}"""
if viewport_only
else ""
)
return f"""
const holders = document.querySelectorAll('{CHART_HOLDER_SELECTOR}');
const unready = [];
for (const holder of holders) {{
const r = holder.getBoundingClientRect();
if (!(r.top < window.innerHeight && r.bottom > 0)) {{
continue;
}}
for (const holder of holders) {{{viewport_skip}
const hasSliceContainer = holder.querySelector(
'{SLICE_CONTAINER_SELECTOR}'
) !== null;
@@ -206,6 +239,13 @@ UNREADY_CHART_HOLDERS_JS_BODY = f"""
}}
"""
# Viewport-scoped scan (tiled path + thumbnails).
UNREADY_CHART_HOLDERS_JS_BODY = _unready_chart_holders_js_body(viewport_only=True)
# Full-dashboard scan (non-tiled report capture, which screenshots the whole
# element in one shot and therefore cannot ignore below-the-fold holders).
UNREADY_ALL_CHART_HOLDERS_JS_BODY = _unready_chart_holders_js_body(viewport_only=False)
# Diagnostic query for every chart holder, including terminal and virtualized
# states. It interpolates the same selector constants as the predicates.
FIND_CHART_HOLDER_STATES_JS = f"""
@@ -256,12 +296,24 @@ REPORT_CHART_HOLDERS_READY_JS = (
f"() => {{ {UNREADY_CHART_HOLDERS_JS_BODY} "
"return holders.length > 0 && unready.length === 0; }"
)
# Report readiness for the non-tiled full-page capture: every mounted holder --
# including below-the-fold ones -- must be terminally rendered. Off-screen
# holders are forced to render first (FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS); if any
# still fails to render within budget the wait times out and the report fails
# loudly rather than shipping a blank/partial screenshot as a Success.
REPORT_ALL_CHART_HOLDERS_READY_JS = (
f"() => {{ {UNREADY_ALL_CHART_HOLDERS_JS_BODY} "
"return holders.length > 0 && unready.length === 0; }"
)
CHART_HOLDERS_MOUNTED_JS = (
f"() => document.querySelectorAll('{CHART_HOLDER_SELECTOR}').length > 0"
)
FIND_UNREADY_CHART_HOLDERS_JS = (
f"() => {{ {UNREADY_CHART_HOLDERS_JS_BODY} return unready; }}"
)
FIND_ALL_UNREADY_CHART_HOLDERS_JS = (
f"() => {{ {UNREADY_ALL_CHART_HOLDERS_JS_BODY} return unready; }}"
)
# A chart capture has one target rather than dashboard holders, but needs the
# same positive terminal-state guarantee and loading exclusion.
+43 -5
View File
@@ -34,8 +34,10 @@ from superset.utils.screenshot_utils import (
CHART_CONTAINER_READY_JS,
CHART_CONTAINER_STATE_JS,
CHART_HOLDERS_READY_JS,
FIND_ALL_UNREADY_CHART_HOLDERS_JS,
FIND_CHART_HOLDER_STATES_JS,
REPORT_CHART_HOLDERS_READY_JS,
FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS,
REPORT_ALL_CHART_HOLDERS_READY_JS,
resolve_screenshot_task_budget_seconds,
ScreenshotTaskBudgetExceededError,
take_tiled_screenshot,
@@ -382,7 +384,19 @@ class WebDriverPlaywright(WebDriverProxy):
if element_name == "chart-container":
readiness_predicate = CHART_CONTAINER_READY_JS
elif report_execution_context:
readiness_predicate = REPORT_CHART_HOLDERS_READY_JS
# This non-tiled path captures the whole element in one shot
# (`_get_screenshot` uses `full_page=True` / `element.screenshot()`),
# so below-the-fold holders end up in the image. Force every
# virtualized row to render up front -- mirroring the client-side
# "Download as Image/PDF" path -- and then require *all* mounted
# holders (not just the viewport-visible ones) to reach a terminal
# state. If an off-screen holder never renders, the wait times out
# and the report fails loudly instead of silently delivering a
# blank/partial screenshot as a Success. The tiled path keeps the
# viewport-scoped predicate because it scrolls each region into view
# before capturing it.
page.evaluate(FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS)
readiness_predicate = REPORT_ALL_CHART_HOLDERS_READY_JS
else:
# Preserve the thumbnail behavior introduced by #42253. The
# stricter zero-holder gate is report-specific because an empty
@@ -430,6 +444,16 @@ class WebDriverPlaywright(WebDriverProxy):
ready_holders = sum(
holder.get("state") in ready_states for holder in chart_holder_states
)
# `FIND_CHART_HOLDER_STATES_JS` short-circuits off-screen holders to
# "virtualized" (counted as ready above), so on the report path -- a
# full-page capture that includes below-the-fold holders -- the real
# culprits (off-screen holders that never rendered) would be hidden.
# Surface them explicitly using the non-viewport-scoped scan.
below_fold_unready = (
page.evaluate(FIND_ALL_UNREADY_CHART_HOLDERS_JS)
if report_execution_context
else unready_chart_holders
)
deadline_elapsed = deadline.elapsed_seconds if deadline else elapsed
deadline_remaining = (
deadline.remaining_seconds if deadline else remaining_budget
@@ -438,7 +462,8 @@ class WebDriverPlaywright(WebDriverProxy):
"report_readiness_terminal url=%s expected_holders=%s "
"mounted_holders=%s ready_holders=%s elapsed_seconds=%.2f "
"remaining_seconds=%s effective_wait_seconds=%.2f%s "
"terminal_reason=readiness_timeout unready_holders=%s states=%s; "
"terminal_reason=readiness_timeout unready_holders=%s "
"all_unready_holders=%s states=%s; "
"aborting before capture or delivery",
url,
expected_holders,
@@ -453,6 +478,7 @@ class WebDriverPlaywright(WebDriverProxy):
effective_load_wait,
context_suffix,
unready_chart_holders,
below_fold_unready,
chart_holder_states,
)
raise
@@ -747,9 +773,21 @@ class WebDriverPlaywright(WebDriverProxy):
context_suffix,
)
# Use tiled screenshots for large dashboards
# Use tiled screenshots for large dashboards. For scheduled
# reports a likely-large dashboard whose measured height is
# at or below a single tile is almost always mid-layout
# (charts still virtualized/collapsed at measurement time),
# not genuinely short -- a 52-chart dashboard is never really
# <one viewport tall. Routing it to the single-shot,
# full-page non-tiled capture risks shipping a windowed
# partial render. Prefer the tiled path, which scrolls every
# region into view and waits per tile; worst case it is a
# single tile. The tiled decision for thumbnails is
# unchanged.
use_tiled = likely_large_dashboard and (
height_unknown or dashboard_height > tile_height
height_unknown
or dashboard_height > tile_height
or report_execution_context is not None
)
if use_tiled:
@@ -14,7 +14,13 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
from superset.utils.pandas_postprocessing import escape_separator, unescape_separator
import inspect
from superset.utils.pandas_postprocessing import (
escape_separator,
pivot,
unescape_separator,
)
def test_escape_separator():
@@ -28,3 +34,19 @@ def test_escape_separator():
escape_string = escape_separator("hello,world")
assert escape_string == r"hello\,world"
assert unescape_separator(escape_string) == "hello,world"
def test_validate_column_args_preserves_signature():
"""
The decorator must not hide the signature of the operation it wraps.
`inspect.signature` follows `__wrapped__`, which `functools.wraps` sets.
Without it every decorated operation reports `(df, **options)`, and code
that inspects the signature -- see `QueryObject._drop_unsupported_options`
-- cannot tell a supported option from an unsupported one.
"""
parameters = inspect.signature(pivot).parameters
assert pivot.__name__ == "pivot"
assert "options" not in parameters
assert {"index", "aggregates", "columns"} <= set(parameters)
@@ -22,6 +22,7 @@ from superset.common.query_object import QueryObject
from superset.connectors.sqla.models import SqlaTable
from superset.models.core import Database
from superset.superset_typing import Metric
from superset.utils import pandas_postprocessing
from superset.utils.core import override_user
@@ -438,3 +439,143 @@ def test_cache_key_cache_impersonation_on_with_different_user_and_db_impersonati
],
any_order=True,
)
def test_post_processing_drops_unsupported_options():
"""
An option that the operation no longer accepts is dropped, not passed on.
A chart saved by an older version of Superset stores `flatten_columns` in
the options of its `pivot` operation. `pivot` lost that parameter when
flattening became its own operation, so replaying the stored query_context
raised `TypeError: pivot() got an unexpected keyword argument
'flatten_columns'`.
"""
query_object = QueryObject(
row_limit=1,
post_processing=[
{
"operation": "pivot",
"options": {
"index": ["__timestamp"],
"columns": ["genre"],
"aggregates": {"count": {"operator": "mean"}},
"drop_missing_columns": False,
"flatten_columns": True,
"reset_index": True,
},
}
],
)
options = query_object.post_processing[0]["options"]
assert "flatten_columns" not in options
assert "reset_index" not in options
assert options["drop_missing_columns"] is False
assert options["index"] == ["__timestamp"]
def test_post_processing_keeps_supported_options():
"""Options the operation accepts are left alone."""
post_processing = [
{
"operation": "pivot",
"options": {"index": ["__timestamp"], "aggregates": {}},
}
]
query_object = QueryObject(row_limit=1, post_processing=post_processing)
assert query_object.post_processing == post_processing
def test_post_processing_keeps_unknown_operation():
"""
An unknown operation is kept, so that `exec_post_processing` can report it
as an `InvalidPostProcessingError` rather than being silently dropped here.
"""
query_object = QueryObject(
row_limit=1,
post_processing=[{"operation": "does_not_exist", "options": {"a": 1}}, None],
)
assert query_object.post_processing == [
{"operation": "does_not_exist", "options": {"a": 1}}
]
def test_post_processing_drops_the_dataframe_parameter():
"""
The DataFrame parameter is not an option.
`exec_post_processing` calls `operation(df, **options)`, so an option named
after the first parameter would raise `TypeError: pivot() got multiple
values for argument 'df'`.
"""
query_object = QueryObject(
row_limit=1,
post_processing=[
{
"operation": "pivot",
"options": {"df": "malformed", "index": ["a"], "aggregates": {}},
}
],
)
options = query_object.post_processing[0]["options"]
assert "df" not in options
assert options["index"] == ["a"]
def test_post_processing_keeps_options_of_a_variadic_operation():
"""An operation that accepts `**kwargs` accepts every option."""
def variadic(df, **kwargs):
return df
post_processing = [{"operation": "variadic", "options": {"anything": 1}}]
with patch.object(pandas_postprocessing, "variadic", variadic, create=True):
query_object = QueryObject(row_limit=1, post_processing=post_processing)
assert query_object.post_processing == post_processing
def test_post_processing_drops_a_variadic_positional_option():
"""
A `*args` parameter cannot be filled by a keyword argument.
`exec_post_processing` calls the operation as `operation(df, **options)`,
so an option named after a `*args` parameter would raise `TypeError:
variadic_positional() got an unexpected keyword argument 'args'` even
though the name appears in the signature.
"""
def variadic_positional(df, *args, index=None): # pylint: disable=unused-argument
return df
with patch.object(
pandas_postprocessing, "variadic_positional", variadic_positional, create=True
):
query_object = QueryObject(
row_limit=1,
post_processing=[
{
"operation": "variadic_positional",
"options": {"args": [1], "index": ["a"]},
}
],
)
options = query_object.post_processing[0]["options"]
assert "args" not in options
assert options["index"] == ["a"]
def test_post_processing_keeps_an_entry_without_an_operation():
"""
An entry that names no operation is kept, so that `exec_post_processing`
reports it as an `InvalidPostProcessingError`.
"""
post_processing = [{"options": {"a": 1}}]
query_object = QueryObject(row_limit=1, post_processing=post_processing)
assert query_object.post_processing == post_processing
+182
View File
@@ -637,6 +637,91 @@ class TestWebDriverPlaywrightErrorHandling:
"",
)
@patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
@patch("superset.utils.webdriver._browser_manager")
@patch("superset.utils.webdriver.take_tiled_screenshot")
def test_large_report_dashboard_tiles_even_when_measured_height_is_short(
self, mock_take_tiled, mock_browser_manager
):
"""Regression: the GOOD (full) report path is the tiled one; BAD
(blank/partial) runs mis-route a large dashboard to the single-shot
non-tiled capture because ``scrollHeight`` is measured while charts are
still virtualized/collapsed (<= one tile). A scheduled report whose
dashboard is large by chart count must take the tiled path regardless
of that stale height measurement, so every region is scrolled into
view and waited on instead of captured as a windowed partial.
"""
mock_user = MagicMock()
mock_user.username = "test_user"
mock_browser = MagicMock()
mock_context = MagicMock()
mock_page = MagicMock()
mock_element = MagicMock()
mock_chart_container = MagicMock()
mock_browser_manager.get_browser.return_value = mock_browser
mock_browser.new_context.return_value = mock_context
mock_context.new_page.return_value = mock_page
def locator_side_effect(selector):
if selector == ".chart-container":
locator = MagicMock()
locator.all.return_value = [mock_chart_container]
return locator
return mock_element
mock_page.locator.side_effect = locator_side_effect
mock_take_tiled.return_value = b"tiled_screenshot"
def evaluate_side_effect(script):
if script == 'document.querySelectorAll(".chart-container").length':
return 52 # mounted containers
if "const target = document.querySelector" in script:
# Non-zero but <= one tile: the classic mid-layout measurement
# that previously vetoed tiling and dropped to the non-tiled
# path.
return 1500
return None
mock_page.evaluate.side_effect = evaluate_side_effect
with patch("superset.utils.webdriver.app") as mock_app:
mock_app.config = {
"WEBDRIVER_OPTION_ARGS": [],
"WEBDRIVER_WINDOW": {"pixel_density": 1},
"SCREENSHOT_PLAYWRIGHT_DEFAULT_TIMEOUT": 30000,
"SCREENSHOT_PLAYWRIGHT_WAIT_EVENT": "networkidle",
"SCREENSHOT_SELENIUM_HEADSTART": 1,
"SCREENSHOT_SELENIUM_ANIMATION_WAIT": 1,
"SCREENSHOT_LOCATE_WAIT": 10,
"SCREENSHOT_LOAD_WAIT": 10,
"SCREENSHOT_REPLACE_UNEXPECTED_ERRORS": False,
"SCREENSHOT_TILED_ENABLED": True,
"SCREENSHOT_TILED_CHART_THRESHOLD": 20,
"SCREENSHOT_TILED_HEIGHT_THRESHOLD": 5000,
# Larger than the measured 1500px height, so only the new
# report-mode branch (not `dashboard_height > tile_height`) can
# select tiling here.
"SCREENSHOT_TILED_VIEWPORT_HEIGHT": 2000,
}
with patch.object(WebDriverPlaywright, "auth") as mock_auth:
mock_auth.return_value = mock_context
driver = WebDriverPlaywright("chrome")
result = driver.get_screenshot(
"http://example.com/dashboard/805",
"standalone",
mock_user,
report_execution_context=_report_context(),
)
assert result == b"tiled_screenshot"
mock_take_tiled.assert_called_once()
# The non-tiled single-shot capture must not run for this large report.
mock_page.screenshot.assert_not_called()
@patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
@patch("superset.utils.webdriver._browser_manager")
@patch("superset.utils.webdriver.logger")
@@ -1168,6 +1253,103 @@ class TestWebDriverPlaywrightChartReadiness:
page.wait_for_function.assert_not_called()
page.screenshot.assert_not_called()
def test_report_readiness_forces_below_fold_render_and_waits_for_all_holders(
self,
):
"""Regression for blank/partial report PDFs.
The non-tiled report capture takes a single full-page screenshot that
includes below-the-fold holders, so the readiness gate must (a) force
every virtualized row to render up front and (b) require *all* mounted
holders -- not just the viewport-visible ones -- to reach a terminal
state. Otherwise an off-screen holder that never rendered is captured
blank and silently delivered as a Success.
"""
from superset.utils.screenshot_utils import (
FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS,
REPORT_ALL_CHART_HOLDERS_READY_JS,
)
page = MagicMock()
page.evaluate.return_value = [{"chartId": "7", "state": "rendered"}]
WebDriverPlaywright._wait_for_charts_ready(
page,
"http://example.com/dashboard/805",
5,
"standalone",
report_execution_context=_report_context(),
)
# (a) Off-screen rows are forced to render before the wait.
assert any(
call.args and call.args[0] == FORCE_ALL_CHART_HOLDERS_IN_VIEW_JS
for call in page.evaluate.call_args_list
)
# (b) The readiness predicate is the all-holders variant: it must not
# skip below-the-fold holders (no viewport-intersection test), so a
# virtualized/unrendered off-screen holder cannot satisfy the gate.
predicate = page.wait_for_function.call_args.args[0]
assert predicate == REPORT_ALL_CHART_HOLDERS_READY_JS
assert "getBoundingClientRect" not in predicate
assert "window.innerHeight" not in predicate
@patch("superset.utils.webdriver.logger")
def test_report_readiness_below_fold_unrendered_fails_loudly(self, mock_logger):
"""When an off-screen holder never renders within budget the report
must fail loudly (raise) rather than capture/deliver a blank
screenshot, and the terminal log must surface the below-the-fold
unready holders that the viewport-scoped diagnostic hides as
'virtualized'.
"""
from superset.utils.screenshot_utils import (
FIND_ALL_UNREADY_CHART_HOLDERS_JS,
FIND_CHART_HOLDER_STATES_JS,
)
from superset.utils.webdriver import PlaywrightTimeout
page = MagicMock()
# 22 on-screen rendered holders + 30 off-screen holders that the
# viewport-scoped diagnostic labels 'virtualized' (and would otherwise
# count as "ready").
holder_states = [
{"chartId": str(i), "state": "rendered"} for i in range(22)
] + [{"chartId": str(i), "state": "virtualized"} for i in range(22, 52)]
below_fold_unready = [
{"chartId": str(i), "state": "nothing_mounted"} for i in range(22, 52)
]
def _evaluate(script, *args):
if script == FIND_ALL_UNREADY_CHART_HOLDERS_JS:
return below_fold_unready
if script == FIND_CHART_HOLDER_STATES_JS:
return holder_states
return None
page.evaluate.side_effect = _evaluate
page.wait_for_function.side_effect = PlaywrightTimeout(
"below-fold holders never rendered"
)
with pytest.raises(PlaywrightTimeout):
WebDriverPlaywright._wait_for_charts_ready(
page,
"http://example.com/dashboard/805",
5,
"standalone",
report_execution_context=_report_context(),
)
terminal_call = next(
call
for call in mock_logger.warning.call_args_list
if call.args and call.args[0].startswith("report_readiness_terminal")
)
assert "terminal_reason=readiness_timeout" in terminal_call.args[0]
# The below-the-fold offenders are surfaced explicitly.
assert "all_unready_holders=" in terminal_call.args[0]
assert below_fold_unready in terminal_call.args
@patch("superset.utils.webdriver.logger")
def test_chart_capture_ready_logs_container_state_not_holder_counts(
self, mock_logger