mirror of
https://github.com/apache/superset.git
synced 2026-08-01 03:22:26 +00:00
Compare commits
32 Commits
codex/ci-m
...
chart-samp
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7eb872f70a | ||
|
|
b2d4c16174 | ||
|
|
e13fb9ad48 | ||
|
|
887ae8a809 | ||
|
|
15d842a68b | ||
|
|
6929d032b8 | ||
|
|
f6c574edd8 | ||
|
|
b452c1634d | ||
|
|
1bfbce3cfd | ||
|
|
85bab0b07c | ||
|
|
0981b1101a | ||
|
|
f607e17e3a | ||
|
|
6c8763bf5a | ||
|
|
035eaa8b80 | ||
|
|
ec8405aeea | ||
|
|
dc632a9737 | ||
|
|
e391691328 | ||
|
|
a419a2a47f | ||
|
|
263d793b77 | ||
|
|
cd77d13cfe | ||
|
|
268662fa49 | ||
|
|
c0f9dec963 | ||
|
|
8e08a65abb | ||
|
|
1a7f2afae9 | ||
|
|
05f45863cb | ||
|
|
96a12e0442 | ||
|
|
06effe2961 | ||
|
|
8235d0c4fb | ||
|
|
67d05d0ed7 | ||
|
|
837ae95b7b | ||
|
|
3692ac5870 | ||
|
|
1df022b364 |
1
.github/actions/chart-releaser-action
vendored
1
.github/actions/chart-releaser-action
vendored
Submodule .github/actions/chart-releaser-action deleted from a917fd15b2
2
.github/workflows/check-python-deps.yml
vendored
2
.github/workflows/check-python-deps.yml
vendored
@@ -46,7 +46,7 @@ jobs:
|
||||
- name: Login to Docker Hub
|
||||
if: steps.check.outputs.python
|
||||
continue-on-error: true
|
||||
uses: docker/login-action@06fb636fac595d6fb4b28a5dfcb21a6f5091859c # v4.5.0
|
||||
uses: docker/login-action@abd2ef45e78c5afb21d64d4ca52ee8550d9572c7 # v4.5.1
|
||||
with:
|
||||
username: ${{ secrets.DOCKERHUB_USER }}
|
||||
password: ${{ secrets.DOCKERHUB_TOKEN }}
|
||||
|
||||
63
.github/workflows/dependabot-auto-approve.yml
vendored
Normal file
63
.github/workflows/dependabot-auto-approve.yml
vendored
Normal file
@@ -0,0 +1,63 @@
|
||||
name: Auto-approve Dependabot patch bumps
|
||||
|
||||
# Posts an approving review on Dependabot PRs that only bump a patch
|
||||
# version, using the same trigger/guard convention already proven to work
|
||||
# for Dependabot PRs in sync-requirements-for-python-dep-upgrade-pr.yml
|
||||
# (plain `pull_request` gets a working, write-capable GITHUB_TOKEN here
|
||||
# because Dependabot pushes branches directly to this repo, not a fork).
|
||||
#
|
||||
# This does NOT auto-merge anything - repo-wide auto-merge is disabled
|
||||
# (Settings > General > Pull Requests > "Allow auto-merge" is off), and
|
||||
# flipping that is a separate, repo-wide decision this workflow doesn't
|
||||
# make on its own. Branch protection also still requires 1 approving
|
||||
# review; this just means that review can already exist by the time a
|
||||
# human looks at the PR, for the (large majority of) ecosystems whose
|
||||
# files aren't matched by any CODEOWNERS pattern. One ecosystem - the npm
|
||||
# bump under .github/actions - matches the /.github/ CODEOWNERS entry, so
|
||||
# those PRs will still need a human owner's approval regardless of this
|
||||
# workflow; it posts a review there too, but that alone won't satisfy the
|
||||
# code-owner requirement.
|
||||
on:
|
||||
pull_request:
|
||||
types: [opened, synchronize]
|
||||
|
||||
# Cancel a superseded run if Dependabot pushes to the same PR again before
|
||||
# the previous run finished (matches the pattern used elsewhere in
|
||||
# superset-docs-verify.yml).
|
||||
concurrency:
|
||||
group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
|
||||
cancel-in-progress: true
|
||||
|
||||
permissions: {}
|
||||
|
||||
jobs:
|
||||
approve-patch-bump:
|
||||
name: Approve patch-level bump
|
||||
# Mirrors the guard in sync-requirements-for-python-dep-upgrade-pr.yml:
|
||||
# limited to (1) PRs authored by Dependabot and (2) the upstream repo,
|
||||
# since forked PRs don't get a write-capable token here anyway.
|
||||
if: >
|
||||
github.repository == 'apache/superset' &&
|
||||
github.event.pull_request.user.login == 'dependabot[bot]' &&
|
||||
github.event.pull_request.head.repo.fork == false
|
||||
runs-on: ubuntu-latest
|
||||
permissions:
|
||||
pull-requests: write # to post the approving review via `gh pr review`
|
||||
steps:
|
||||
- name: Fetch Dependabot metadata
|
||||
id: metadata
|
||||
# This exact SHA is on ASF Infra's action allowlist
|
||||
# (apache/infrastructure-actions approved_patterns.yml) as of this
|
||||
# writing. Do not bump without opening an Infra ticket to allow
|
||||
# the new SHA first!
|
||||
uses: dependabot/fetch-metadata@25dd0e34f4fe68f24cc83900b1fe3fe149efef98 # v3.1.0
|
||||
|
||||
- name: Approve patch-level bump
|
||||
if: steps.metadata.outputs.update-type == 'version-update:semver-patch'
|
||||
env:
|
||||
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
PR_URL: ${{ github.event.pull_request.html_url }}
|
||||
DEPENDENCY_NAMES: ${{ steps.metadata.outputs.dependency-names }}
|
||||
run: |
|
||||
gh pr review --approve "$PR_URL" \
|
||||
--body "Auto-approved: patch-level bump only ($DEPENDENCY_NAMES)."
|
||||
113
.github/workflows/mirror-service-images.yml
vendored
Normal file
113
.github/workflows/mirror-service-images.yml
vendored
Normal file
@@ -0,0 +1,113 @@
|
||||
# Licensed to the Apache Software Foundation (ASF) under one
|
||||
# or more contributor license agreements. See the NOTICE file
|
||||
# distributed with this work for additional information
|
||||
# regarding copyright ownership. The ASF licenses this file
|
||||
# to you under the Apache License, Version 2.0 (the
|
||||
# "License"); you may not use this file except in compliance
|
||||
# with the License. You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing,
|
||||
# software distributed under the License is distributed on an
|
||||
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
# KIND, either express or implied. See the License for the
|
||||
# specific language governing permissions and limitations
|
||||
# under the License.
|
||||
#
|
||||
# Mirror the Docker Hub service-container images that CI relies on into the
|
||||
# repository's GitHub Container Registry (GHCR) namespace.
|
||||
#
|
||||
# WHY: CI jobs declare Postgres/MySQL/Redis/Presto as `services:` containers
|
||||
# pulled anonymously from Docker Hub. Anonymous pulls share the runner's IP
|
||||
# rate limit, which causes intermittent timeouts / 429s / 502s on `master`
|
||||
# and same-repo PRs. The obvious fix — adding `credentials:` to the service
|
||||
# blocks — breaks fork PRs hard: forks can't read secrets, so the templated
|
||||
# username/password resolve to '' and GitHub rejects the workflow at parse
|
||||
# time ("Unexpected value ''"), failing every fork job at "Set up job".
|
||||
#
|
||||
# Mirroring to GHCR sidesteps both problems: public GHCR images are pulled
|
||||
# without Docker Hub's anonymous rate limit AND without any credentials, so
|
||||
# the consuming workflows need no `credentials:` block and forks work
|
||||
# unchanged.
|
||||
#
|
||||
# ONE-TIME BOOTSTRAP (maintainer, after this lands on the default branch):
|
||||
# 1. Run this workflow once (Actions tab → "Mirror service images to GHCR"
|
||||
# → Run workflow), or wait for the weekly schedule.
|
||||
# 2. In the org's Packages settings, set each mirrored package's visibility
|
||||
# to **public** (apache/superset → ci/postgres, ci/mysql, ci/redis,
|
||||
# ci/presto). Public visibility is what lets fork CI pull without auth.
|
||||
# 3. Only then merge the follow-up that repoints the `services.*.image`
|
||||
# refs at these GHCR copies and drops the `credentials:` blocks.
|
||||
#
|
||||
# NOTE: this mirrors only the images declared as `services:` containers (the
|
||||
# ones that broke forks). The `bde2020` hive-metastore image pulled via
|
||||
# `docker compose` in the Presto/Hive job is a separate path and is left for
|
||||
# a follow-up.
|
||||
|
||||
name: Mirror service images to GHCR
|
||||
|
||||
on:
|
||||
schedule:
|
||||
# Weekly, Monday 06:00 UTC — keeps the mirror fresh as upstream tags move.
|
||||
- cron: "0 6 * * 1"
|
||||
workflow_dispatch: {}
|
||||
|
||||
concurrency:
|
||||
group: mirror-service-images
|
||||
cancel-in-progress: false
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
packages: write
|
||||
|
||||
jobs:
|
||||
mirror:
|
||||
# Never run on forks: they lack both the secrets and write access to the
|
||||
# apache GHCR namespace, so a scheduled run there would only ever fail.
|
||||
if: github.repository == 'apache/superset'
|
||||
runs-on: ubuntu-latest
|
||||
strategy:
|
||||
fail-fast: false
|
||||
matrix:
|
||||
# Keep this list in sync with the `services.*.image` refs in
|
||||
# superset-e2e.yml, superset-python-integrationtest.yml, and
|
||||
# superset-python-presto-hive.yml.
|
||||
image:
|
||||
- postgres:17-alpine
|
||||
- redis:7-alpine
|
||||
- mysql:8.0
|
||||
- starburstdata/presto:350-e.6
|
||||
steps:
|
||||
- name: Log in to Docker Hub (authenticated source pulls)
|
||||
uses: docker/login-action@abd2ef45e78c5afb21d64d4ca52ee8550d9572c7 # v4.5.1
|
||||
with:
|
||||
username: ${{ secrets.DOCKERHUB_USER }}
|
||||
password: ${{ secrets.DOCKERHUB_TOKEN }}
|
||||
|
||||
- name: Log in to GHCR (push target)
|
||||
uses: docker/login-action@abd2ef45e78c5afb21d64d4ca52ee8550d9572c7 # v4.5.1
|
||||
with:
|
||||
registry: ghcr.io
|
||||
username: ${{ github.actor }}
|
||||
password: ${{ secrets.GITHUB_TOKEN }}
|
||||
|
||||
- name: Set up Docker Buildx
|
||||
uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0
|
||||
|
||||
- name: Copy image to GHCR
|
||||
env:
|
||||
# Pass the matrix value through the environment rather than
|
||||
# interpolating it into the shell, to avoid template injection.
|
||||
SRC_IMAGE: ${{ matrix.image }}
|
||||
run: |
|
||||
set -euo pipefail
|
||||
# Destination keeps the image's short name (drop any namespace),
|
||||
# under a `ci/` prefix in this repo's GHCR namespace.
|
||||
name="${SRC_IMAGE##*/}"
|
||||
dst="ghcr.io/${GITHUB_REPOSITORY}/ci/${name}"
|
||||
echo "Mirroring docker.io/${SRC_IMAGE} -> ${dst}"
|
||||
# imagetools copies the full (multi-arch) manifest registry-to-
|
||||
# registry without a local pull/retag/push round trip.
|
||||
docker buildx imagetools create --tag "${dst}" "docker.io/${SRC_IMAGE}"
|
||||
echo "- \`docker.io/${SRC_IMAGE}\` → \`${dst}\`" >> "${GITHUB_STEP_SUMMARY}"
|
||||
@@ -38,6 +38,11 @@ jobs:
|
||||
with:
|
||||
install-superset: "false"
|
||||
|
||||
# Still vendored (not de-vendored like chart-releaser-action below): the
|
||||
# allowlisted helm/chart-testing-action@v2.8.0 depends internally on
|
||||
# astral-sh/setup-uv@v7.0.0, which isn't itself on the ASF Actions
|
||||
# allowlist (only v8.1.0+ are, at apache/infrastructure-actions'
|
||||
# actions.yml). Needs an INFRA request before this can de-vendor too.
|
||||
- name: Set up chart-testing
|
||||
uses: ./.github/actions/chart-testing-action
|
||||
|
||||
|
||||
7
.github/workflows/superset-helm-release.yml
vendored
7
.github/workflows/superset-helm-release.yml
vendored
@@ -95,13 +95,8 @@ jobs:
|
||||
# Return to the original branch
|
||||
git checkout local_gha_temp
|
||||
|
||||
- name: Fetch/list all tags
|
||||
run: |
|
||||
git submodule update
|
||||
cat .github/actions/chart-releaser-action/action.yml
|
||||
|
||||
- name: Run chart-releaser
|
||||
uses: ./.github/actions/chart-releaser-action
|
||||
uses: helm/chart-releaser-action@cae68fefc6b5f367a0275617c9f83181ba54714f # v1.7.0
|
||||
with:
|
||||
version: v1.6.0
|
||||
charts_dir: helm
|
||||
|
||||
@@ -38,7 +38,7 @@ jobs:
|
||||
- name: Login to Docker Hub
|
||||
if: ${{ steps.dependabot-metadata.outputs.package-ecosystem == 'pip' }}
|
||||
continue-on-error: true
|
||||
uses: docker/login-action@06fb636fac595d6fb4b28a5dfcb21a6f5091859c # v4.5.0
|
||||
uses: docker/login-action@abd2ef45e78c5afb21d64d4ca52ee8550d9572c7 # v4.5.1
|
||||
with:
|
||||
username: ${{ secrets.DOCKERHUB_USER }}
|
||||
password: ${{ secrets.DOCKERHUB_TOKEN }}
|
||||
|
||||
3
.gitmodules
vendored
3
.gitmodules
vendored
@@ -30,9 +30,6 @@
|
||||
[submodule ".github/actions/chart-testing-action"]
|
||||
path = .github/actions/chart-testing-action
|
||||
url = https://github.com/helm/chart-testing-action
|
||||
[submodule ".github/actions/chart-releaser-action"]
|
||||
path = .github/actions/chart-releaser-action
|
||||
url = https://github.com/helm/chart-releaser-action
|
||||
[submodule ".github/actions/github-action-push-to-another-repository"]
|
||||
path = .github/actions/github-action-push-to-another-repository
|
||||
url = https://github.com/cpina/github-action-push-to-another-repository
|
||||
|
||||
@@ -423,7 +423,7 @@ git push origin ${SUPERSET_VERSION}
|
||||
|
||||
### Publishing a Convenience Release to PyPI
|
||||
|
||||
Extract the release to the `/tmp` folder to build the PiPY release. Files in the `/tmp` folder will be automatically deleted by the OS.
|
||||
Extract the release to the `/tmp` folder to build the PyPI release. Files in the `/tmp` folder will be automatically deleted by the OS.
|
||||
|
||||
```bash
|
||||
mkdir -p /tmp/superset && cd /tmp/superset
|
||||
|
||||
@@ -112,12 +112,61 @@ USER superset
|
||||
CMD ["/app/docker/entrypoints/run-server.sh"]
|
||||
```
|
||||
|
||||
### Adding translations to a custom image
|
||||
|
||||
The pattern above, a small Dockerfile that just extends `FROM apache/superset:...`, can't add
|
||||
translations after the fact. By the time an official tag is published, its frontend and backend
|
||||
layers have already had non-English translation files stripped out unless `BUILD_TRANSLATIONS`
|
||||
was set at build time (see below), and there's no `superset/translations` source tree left in the
|
||||
final image to compile from.
|
||||
|
||||
To get translations into your own image, you need to build from the full Superset source (a
|
||||
clone or fork of this repo) rather than extend a published tag. The most efficient way to do this
|
||||
is to append your customizations as one more stage at the end of the repo's own `Dockerfile`, so
|
||||
Docker can reuse the cached upstream layers and only rebuild what your stage adds:
|
||||
|
||||
```Dockerfile
|
||||
# Append this to the end of the repo's Dockerfile
|
||||
# Keep this tag in sync with the branch/tag of the repo you cloned, so the
|
||||
# translation files built from source match the keys the runtime expects:
|
||||
FROM apache/superset:5.0.0 AS my-custom-image
|
||||
USER root
|
||||
|
||||
# Pull the translation files out of the earlier build stages (frontend
|
||||
# .json in `superset-node`, backend .mo in `python-translation-compiler`).
|
||||
# Those stages' own cleanup only matches single-character extensions, so
|
||||
# the source `.po` files can still be present here; strip them explicitly
|
||||
# so this stage only keeps the compiled translations.
|
||||
COPY --from=superset-node /app/superset/translations superset/translations
|
||||
COPY --from=python-translation-compiler /app/translations_mo superset/translations
|
||||
RUN find superset/translations -name '*.po' -delete
|
||||
|
||||
USER superset
|
||||
```
|
||||
|
||||
Then build with:
|
||||
|
||||
```bash
|
||||
docker build --target=my-custom-image --build-arg=BUILD_TRANSLATIONS=true -t mysuperset:5.0.0 .
|
||||
```
|
||||
|
||||
You can combine this with the database-driver/dependency pattern above by adding your own
|
||||
`RUN uv pip install ...` step before switching back to `USER superset`. See
|
||||
[issue #35959](https://github.com/apache/superset/issues/35959) for the discussion this pattern
|
||||
came out of, credit to the community for working it out.
|
||||
|
||||
## Key ARGs in Dockerfile
|
||||
|
||||
- `BUILD_TRANSLATIONS`: whether to build the translations into the image. For the
|
||||
frontend build this tells webpack to strip out all locales other than `en` from
|
||||
the `moment-timezone` library. For the backendthis skips compiling the
|
||||
`*.po` translation files
|
||||
- `BUILD_TRANSLATIONS`: whether to compile non-English translations into the image.
|
||||
When `true`, the frontend build converts the `*.po` files to locale JSON and the
|
||||
backend runs `pybabel compile` to produce `*.mo` files; both source `*.po` files
|
||||
are stripped afterward either way. When `false` (the default), those compile
|
||||
steps are skipped and only `en` ships. This only takes effect when building the image from source
|
||||
(`docker build` against this repo's own `Dockerfile`); it has no effect on a downstream
|
||||
Dockerfile that just extends an already-published tag, see
|
||||
"Adding translations to a custom image" above. Note that the backend `pybabel compile`
|
||||
step ignores its exit code, so a `.po` file with a compile error won't fail the build;
|
||||
check the build logs for `pybabel` warnings if a locale's backend strings aren't showing up.
|
||||
- `DEV_MODE`: whether to skip the frontend build, this is used by our `docker-compose` dev setup
|
||||
where we mount the local volume and build using `webpack` in `--watch` mode, meaning as you
|
||||
alter the code in the local file system, webpack, from within a docker image used for this
|
||||
|
||||
@@ -61,7 +61,7 @@
|
||||
"@storybook/addon-docs": "^10.5.3",
|
||||
"@superset-ui/core": "^0.20.4",
|
||||
"@swc/core": "^1.15.46",
|
||||
"antd": "^6.5.1",
|
||||
"antd": "^6.5.2",
|
||||
"baseline-browser-mapping": "^2.11.1",
|
||||
"caniuse-lite": "^1.0.30001806",
|
||||
"docusaurus-plugin-openapi-docs": "^5.1.2",
|
||||
@@ -99,7 +99,7 @@
|
||||
"prettier": "^3.9.6",
|
||||
"typescript": "~6.0.3",
|
||||
"typescript-eslint": "^8.65.0",
|
||||
"webpack": "^5.108.2"
|
||||
"webpack": "^5.109.0"
|
||||
},
|
||||
"browserslist": {
|
||||
"production": [
|
||||
|
||||
@@ -3779,7 +3779,7 @@
|
||||
"@rc-component/virtual-list" "^1.2.0"
|
||||
clsx "^2.1.1"
|
||||
|
||||
"@rc-component/trigger@^3.0.0", "@rc-component/trigger@^3.10.0", "@rc-component/trigger@^3.6.15", "@rc-component/trigger@^3.7.1":
|
||||
"@rc-component/trigger@^3.0.0", "@rc-component/trigger@^3.10.1", "@rc-component/trigger@^3.6.15", "@rc-component/trigger@^3.7.1":
|
||||
version "3.10.1"
|
||||
resolved "https://registry.yarnpkg.com/@rc-component/trigger/-/trigger-3.10.1.tgz#cb28e1bc0745a2af6897dd7ec774f9b56dc88f86"
|
||||
integrity sha512-mXlDN0IXdtV8Yqqm8195ECCyrbmfvvfKvwVvSlH0+qvKD6BUF8gRhEjSy0FOcD1+CcDRHgTiX99LoxfQrmh3Cw==
|
||||
@@ -3798,13 +3798,13 @@
|
||||
"@rc-component/util" "^1.11.1"
|
||||
clsx "^2.1.1"
|
||||
|
||||
"@rc-component/util@^1.10.1", "@rc-component/util@^1.11.0", "@rc-component/util@^1.11.1", "@rc-component/util@^1.2.0", "@rc-component/util@^1.2.1", "@rc-component/util@^1.3.0", "@rc-component/util@^1.4.0", "@rc-component/util@^1.7.0", "@rc-component/util@^1.9.0":
|
||||
version "1.11.1"
|
||||
resolved "https://registry.yarnpkg.com/@rc-component/util/-/util-1.11.1.tgz#07d698908339c55648e4f974afa739345e65b483"
|
||||
integrity sha512-awVlI3ub2vqfqkYxOBc/uQ0efm3jw0wcrhtO/YWLyZfxiKXczKwNbVuhlnyxytDt7H9pbbVQiqr+O6MLATtRYg==
|
||||
"@rc-component/util@^1.10.1", "@rc-component/util@^1.11.0", "@rc-component/util@^1.11.1", "@rc-component/util@^1.12.0", "@rc-component/util@^1.2.0", "@rc-component/util@^1.2.1", "@rc-component/util@^1.3.0", "@rc-component/util@^1.4.0", "@rc-component/util@^1.7.0", "@rc-component/util@^1.9.0":
|
||||
version "1.12.0"
|
||||
resolved "https://registry.yarnpkg.com/@rc-component/util/-/util-1.12.0.tgz#58e453585810bcb8a35ff1aafd5e01187457b86f"
|
||||
integrity sha512-AEjPL8JVdohIITaiXokyjL9WQ6tKWWjAYK9QU16tGNE9JaQABBQy+hA4H2Lup5MgXy9yY3iLrbZJheuU13hTdQ==
|
||||
dependencies:
|
||||
is-mobile "^5.0.0"
|
||||
react-is "^18.2.0"
|
||||
react-is "^19.2.7"
|
||||
|
||||
"@rc-component/virtual-list@^1.0.1", "@rc-component/virtual-list@^1.2.0":
|
||||
version "1.2.0"
|
||||
@@ -5835,11 +5835,6 @@ acorn-dynamic-import@^4.0.0:
|
||||
resolved "https://registry.yarnpkg.com/acorn-dynamic-import/-/acorn-dynamic-import-4.0.0.tgz#482210140582a36b83c3e342e1cfebcaa9240948"
|
||||
integrity sha512-d3OEjQV4ROpoflsnUA8HozoIR504TFxNivYEUi6uwz0IYhBkTDXGuWlNdMtybRt3nqVx/L6XqMt0FxkXuWKZhw==
|
||||
|
||||
acorn-import-phases@^1.0.3:
|
||||
version "1.0.4"
|
||||
resolved "https://registry.yarnpkg.com/acorn-import-phases/-/acorn-import-phases-1.0.4.tgz#16eb850ba99a056cb7cbfe872ffb8972e18c8bd7"
|
||||
integrity sha512-wKmbr/DDiIXzEOiWrTTUcDm24kQ2vGfZQvM2fwg2vXqR5uW6aapr7ObPtj1th32b9u90/Pf4AItvdTh42fBmVQ==
|
||||
|
||||
acorn-jsx@^5.0.0, acorn-jsx@^5.0.1, acorn-jsx@^5.3.2:
|
||||
version "5.3.2"
|
||||
resolved "https://registry.yarnpkg.com/acorn-jsx/-/acorn-jsx-5.3.2.tgz#7ed5bb55908b3b2f1bc55c6af1653bada7f07937"
|
||||
@@ -6028,10 +6023,10 @@ ansis@^3.2.0:
|
||||
resolved "https://registry.yarnpkg.com/ansis/-/ansis-3.17.0.tgz#fa8d9c2a93fe7d1177e0c17f9eeb562a58a832d7"
|
||||
integrity sha512-0qWUglt9JEqLFr3w1I1pbrChn1grhaiAR2ocX1PP/flRmxgtwTzPFFFnfIlD6aMOLQZgSuCRlidD70lvx8yhzg==
|
||||
|
||||
antd@^6.5.1:
|
||||
version "6.5.1"
|
||||
resolved "https://registry.yarnpkg.com/antd/-/antd-6.5.1.tgz#2623db1f3b0ae32a2e311ef20f2f1413934d0094"
|
||||
integrity sha512-VZVVF9zYI6S0NHqboVhCoY9Iiqj6dphW1NPB+sEaAf2HuIQ0haXWXj7ZvAXTRDzusktV6+cvvrSZEdRi4twATg==
|
||||
antd@^6.5.2:
|
||||
version "6.5.2"
|
||||
resolved "https://registry.yarnpkg.com/antd/-/antd-6.5.2.tgz#d771211beddf539f37303f862df24e8aaf32b7c4"
|
||||
integrity sha512-ntYx0lr4Jq192QnBkDWkDqEeoberXZ34vSE9SgiP/0J6DY8O0pzR3bVZLBsdpCSguVkwjtEAP+QNeMN7LNAvgw==
|
||||
dependencies:
|
||||
"@ant-design/colors" "^8.0.1"
|
||||
"@ant-design/cssinjs" "^2.1.2"
|
||||
@@ -6073,9 +6068,9 @@ antd@^6.5.1:
|
||||
"@rc-component/tour" "~2.4.0"
|
||||
"@rc-component/tree" "~1.3.2"
|
||||
"@rc-component/tree-select" "~1.11.0"
|
||||
"@rc-component/trigger" "^3.10.0"
|
||||
"@rc-component/trigger" "^3.10.1"
|
||||
"@rc-component/upload" "~1.1.1"
|
||||
"@rc-component/util" "^1.11.1"
|
||||
"@rc-component/util" "^1.12.0"
|
||||
clsx "^2.1.1"
|
||||
dayjs "^1.11.11"
|
||||
scroll-into-view-if-needed "^3.1.0"
|
||||
@@ -8066,10 +8061,10 @@ encodeurl@~2.0.0:
|
||||
resolved "https://registry.yarnpkg.com/encodeurl/-/encodeurl-2.0.0.tgz#7b8ea898077d7e409d3ac45474ea38eaf0857a58"
|
||||
integrity sha512-Q0n9HRi4m6JuGIV1eFlmvJB7ZEVxu93IrMyiMsGC0lrMJMWzRgx6WGquyfQgZVb31vhGgXnfmPNNXmxnOkRBrg==
|
||||
|
||||
enhanced-resolve@^5.22.2:
|
||||
version "5.24.1"
|
||||
resolved "https://registry.yarnpkg.com/enhanced-resolve/-/enhanced-resolve-5.24.1.tgz#b2439adf5d31d7e4764de1f9ecf942d6cd3fc874"
|
||||
integrity sha512-7DdUaTjmNwMcH2gLr1qycesKII3BK4RLy/mdAb7x10Lq7bR4aNKHt1BR1ZALSv0rPM/hF5wYF0PhGop/rJm8vw==
|
||||
enhanced-resolve@^5.24.2:
|
||||
version "5.24.5"
|
||||
resolved "https://registry.yarnpkg.com/enhanced-resolve/-/enhanced-resolve-5.24.5.tgz#b4dad3255b7545f07ba5535189868e9f85f47573"
|
||||
integrity sha512-L1l8TNvomm6UVW5B253AGxQagSQr+vGwhMlrrfRS2qmhx46AMpMVJKQYLvWYbysTMY8VoicOvzHzoHMbyzB+4A==
|
||||
dependencies:
|
||||
graceful-fs "^4.2.4"
|
||||
tapable "^2.3.3"
|
||||
@@ -10431,11 +10426,6 @@ liquid-json@0.3.1:
|
||||
resolved "https://registry.yarnpkg.com/liquid-json/-/liquid-json-0.3.1.tgz#9155a18136d8a6b2615e5f16f9a2448ab6b50eea"
|
||||
integrity sha512-wUayTU8MS827Dam6MxgD72Ui+KOSF+u/eIqpatOtjnvgJ0+mnDq33uC2M7J0tPK+upe/DpUAuK4JUU89iBoNKQ==
|
||||
|
||||
loader-runner@^4.3.2:
|
||||
version "4.3.2"
|
||||
resolved "https://registry.yarnpkg.com/loader-runner/-/loader-runner-4.3.2.tgz#9913d3a15971f8f635915e601fb5c9d495d918e9"
|
||||
integrity sha512-DFEqQ3ihfS9blba08cLfYf1NRAIEm+dDjic073DRDc3/JspI/8wYmtDsHwd3+4hwvdxSK7PGaElfTmm0awWJ4w==
|
||||
|
||||
loader-utils@^2.0.0:
|
||||
version "2.0.4"
|
||||
resolved "https://registry.yarnpkg.com/loader-utils/-/loader-utils-2.0.4.tgz#8b5cb38b5c34a9a018ee1fc0e6a066d1dfcc528c"
|
||||
@@ -13327,11 +13317,16 @@ react-is@^17.0.1:
|
||||
resolved "https://registry.yarnpkg.com/react-is/-/react-is-17.0.2.tgz#e691d4a8e9c789365655539ab372762b0efb54f0"
|
||||
integrity sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w==
|
||||
|
||||
react-is@^18.0.0, react-is@^18.2.0:
|
||||
react-is@^18.0.0:
|
||||
version "18.3.1"
|
||||
resolved "https://registry.yarnpkg.com/react-is/-/react-is-18.3.1.tgz#e83557dc12eae63a99e003a46388b1dcbb44db7e"
|
||||
integrity sha512-/LLMVyas0ljjAtoYiPqYiL8VWXzUUdThrmU5+n20DZv+a+ClRoevUzw5JxU+Ieh5/c87ytoTBV9G1FiKfNJdmg==
|
||||
|
||||
react-is@^19.2.7:
|
||||
version "19.2.8"
|
||||
resolved "https://registry.yarnpkg.com/react-is/-/react-is-19.2.8.tgz#09826f9fbc187bc668e3e5c62edc001f804d5018"
|
||||
integrity sha512-s5un28nYxKJw5gvUHyW5PCC28CvBqLu9r3cWgzHT4Vo/5fqqkFcdRYsGcKf50WMPpjjFZS5d76fn3YCo2njKwQ==
|
||||
|
||||
react-json-view-lite@^2.3.0:
|
||||
version "2.5.0"
|
||||
resolved "https://registry.yarnpkg.com/react-json-view-lite/-/react-json-view-lite-2.5.0.tgz#c7ff011c7cc80e9900abc7aa4916c6a5c6d6c1c6"
|
||||
@@ -15855,20 +15850,20 @@ webpack-merge@^6.0.1:
|
||||
flat "^5.0.2"
|
||||
wildcard "^2.0.1"
|
||||
|
||||
webpack-sources@^3.5.0:
|
||||
version "3.5.0"
|
||||
resolved "https://registry.yarnpkg.com/webpack-sources/-/webpack-sources-3.5.0.tgz#87bf7f5801a4e985b1f1c92b64b9620a02f76d08"
|
||||
integrity sha512-HPuy+uuoTCaaoEoI1LQ3JN9+vrPBvEesnnX1jADHy728cHSMlq4wUc4afYqahq2B1mhQVZxCXOkNTnXltr+2vQ==
|
||||
webpack-sources@^3.5.1:
|
||||
version "3.5.1"
|
||||
resolved "https://registry.yarnpkg.com/webpack-sources/-/webpack-sources-3.5.1.tgz#76c2418486dcc02b2aa0694c104176c2858fe84a"
|
||||
integrity sha512-jyuiGJdtvY434z5bUZrjz67v76/ePNvFZTp9Mdz29IlH4+GPsgyGjiv0fKI+M7BdkU6ADjulUcKAd3tUK3WlEw==
|
||||
|
||||
webpack-virtual-modules@^0.6.2:
|
||||
version "0.6.2"
|
||||
resolved "https://registry.yarnpkg.com/webpack-virtual-modules/-/webpack-virtual-modules-0.6.2.tgz#057faa9065c8acf48f24cb57ac0e77739ab9a7e8"
|
||||
integrity sha512-66/V2i5hQanC51vBQKPH4aI8NMAcBW59FVBs+rC7eGHupMyfn34q7rZIE+ETlJ+XTevqfUhVVBgSUNSW2flEUQ==
|
||||
|
||||
webpack@^5.108.2, webpack@^5.88.1, webpack@^5.95.0:
|
||||
version "5.108.4"
|
||||
resolved "https://registry.yarnpkg.com/webpack/-/webpack-5.108.4.tgz#141818a411662773a0bb32dc5536acc5409943b7"
|
||||
integrity sha512-yur8LyJoeiWh47dErD+Ok7vlbmDsJ3UbbRPAoxbGJ54WpE2y5yVo5G/inUzujnYgw3tPmBRdn+G7PoxXaYC33w==
|
||||
webpack@^5.109.0, webpack@^5.88.1, webpack@^5.95.0:
|
||||
version "5.109.0"
|
||||
resolved "https://registry.yarnpkg.com/webpack/-/webpack-5.109.0.tgz#871d8eee5e2d5e6eaf5ec8d1a6db74ea65491030"
|
||||
integrity sha512-vomrngskVVXEZF9sMZfYAd4pXZUnfaWdJGlF+BTNF+gJBCKYCQBnOeVPlrh39Ewl7nlCsirDplMy6o5g9xJHBg==
|
||||
dependencies:
|
||||
"@types/estree" "^1.0.8"
|
||||
"@types/json-schema" "^7.0.15"
|
||||
@@ -15876,22 +15871,20 @@ webpack@^5.108.2, webpack@^5.88.1, webpack@^5.95.0:
|
||||
"@webassemblyjs/wasm-edit" "^1.14.1"
|
||||
"@webassemblyjs/wasm-parser" "^1.14.1"
|
||||
acorn "^8.16.0"
|
||||
acorn-import-phases "^1.0.3"
|
||||
browserslist "^4.28.1"
|
||||
chrome-trace-event "^1.0.2"
|
||||
enhanced-resolve "^5.22.2"
|
||||
enhanced-resolve "^5.24.2"
|
||||
es-module-lexer "^2.1.0"
|
||||
eslint-scope "5.1.1"
|
||||
events "^3.2.0"
|
||||
graceful-fs "^4.2.11"
|
||||
loader-runner "^4.3.2"
|
||||
mime-db "^1.54.0"
|
||||
minimizer-webpack-plugin "^5.6.1"
|
||||
neo-async "^2.6.2"
|
||||
schema-utils "^4.3.3"
|
||||
tapable "^2.3.0"
|
||||
watchpack "^2.5.2"
|
||||
webpack-sources "^3.5.0"
|
||||
webpack-sources "^3.5.1"
|
||||
|
||||
webpackbar@^7.0.0:
|
||||
version "7.0.0"
|
||||
|
||||
@@ -215,7 +215,7 @@ Common permissions you might need:
|
||||
|
||||
By default, the Embedded SDK creates an `iframe` element without a `referrerPolicy` value enforced. This means that a policy defined for `iframe` elements at the host app level would reflect to it.
|
||||
|
||||
This can be an issue as during the embedded enablement for a dashboard it's possible to specify which domain(s) are allowed to embed the dashboard, and this validation happens throuth the `Referrer` header. That said, in case the hosting app has a more restrictive policy that would omit this header, this validation would fail.
|
||||
This can be an issue as during the embedded enablement for a dashboard it's possible to specify which domain(s) are allowed to embed the dashboard, and this validation happens through the `Referrer` header. That said, in case the hosting app has a more restrictive policy that would omit this header, this validation would fail.
|
||||
|
||||
Use the `referrerPolicy` parameter in the `embedDashboard` method to specify [a particular policy](https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Referrer-Policy) that works for your implementation.
|
||||
|
||||
|
||||
730
superset-frontend/package-lock.json
generated
730
superset-frontend/package-lock.json
generated
File diff suppressed because it is too large
Load Diff
@@ -178,7 +178,7 @@
|
||||
"dom-to-pdf": "^0.3.2",
|
||||
"echarts": "^6.1.0",
|
||||
"fast-glob": "^3.3.2",
|
||||
"fs-extra": "^11.3.6",
|
||||
"fs-extra": "^11.4.0",
|
||||
"fuse.js": "^7.5.0",
|
||||
"geolib": "^3.3.14",
|
||||
"geostyler": "^18.6.0",
|
||||
@@ -186,7 +186,7 @@
|
||||
"geostyler-openlayers-parser": "^5.7.1",
|
||||
"geostyler-style": "11.0.2",
|
||||
"geostyler-wfs-parser": "^3.0.1",
|
||||
"google-auth-library": "^10.9.0",
|
||||
"google-auth-library": "^10.9.1",
|
||||
"immer": "^11.1.15",
|
||||
"interweave": "^13.1.1",
|
||||
"jquery": "^4.0.0",
|
||||
@@ -195,7 +195,7 @@
|
||||
"json-stringify-pretty-compact": "^4.0.0",
|
||||
"lodash": "^4.18.1",
|
||||
"lodash-es": "^4.18.1",
|
||||
"mapbox-gl": "^3.26.0",
|
||||
"mapbox-gl": "^3.27.0",
|
||||
"markdown-to-jsx": "^9.9.0",
|
||||
"match-sorter": "^8.3.0",
|
||||
"memoize-one": "^6.0.0",
|
||||
@@ -367,7 +367,7 @@
|
||||
"typescript": "5.4.5",
|
||||
"unzipper": "^0.12.5",
|
||||
"wait-on": "^9.1.0",
|
||||
"webpack": "^5.108.4",
|
||||
"webpack": "^5.109.0",
|
||||
"webpack-bundle-analyzer": "^5.3.1",
|
||||
"webpack-cli": "^7.0.3",
|
||||
"webpack-dev-server": "^5.2.5",
|
||||
|
||||
@@ -35,7 +35,7 @@
|
||||
},
|
||||
"devDependencies": {
|
||||
"cross-env": "^10.1.0",
|
||||
"fs-extra": "^11.3.6",
|
||||
"fs-extra": "^11.4.0",
|
||||
"jest": "^30.4.2",
|
||||
"yeoman-test": "^11.6.0"
|
||||
},
|
||||
|
||||
@@ -27,7 +27,7 @@
|
||||
],
|
||||
"dependencies": {
|
||||
"@math.gl/web-mercator": "^4.1.0",
|
||||
"mapbox-gl": "^3.26.0",
|
||||
"mapbox-gl": "^3.27.0",
|
||||
"maplibre-gl": "^5.24.0",
|
||||
"react-map-gl": "^8.1.1",
|
||||
"supercluster": "^8.0.1"
|
||||
|
||||
@@ -216,6 +216,12 @@ test('should render the error', async () => {
|
||||
.spyOn(SupersetClient, 'post')
|
||||
.mockRejectedValue(new Error('Something went wrong'));
|
||||
await waitForRender();
|
||||
// The error is wrapped in an Alert component with a stable headline and the
|
||||
// raw error text in the description — no more bare ``<pre>`` elements.
|
||||
expect(await screen.findByRole('alert')).toBeVisible();
|
||||
expect(
|
||||
await screen.findByText('Failed to load drill-to-detail rows'),
|
||||
).toBeVisible();
|
||||
expect(screen.getByText('Error: Something went wrong')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
|
||||
@@ -42,6 +42,7 @@ import BooleanCell from '@superset-ui/core/components/Table/cell-renderers/Boole
|
||||
import NullCell from '@superset-ui/core/components/Table/cell-renderers/NullCell';
|
||||
import TimeCell from '@superset-ui/core/components/Table/cell-renderers/TimeCell';
|
||||
import { EmptyState, Loading } from '@superset-ui/core/components';
|
||||
import { Alert } from '@apache-superset/core/components';
|
||||
import { getDatasourceSamples } from 'src/components/Chart/chartAction';
|
||||
import Table, {
|
||||
ColumnsType,
|
||||
@@ -362,13 +363,18 @@ export default function DrillDetailPane({
|
||||
if (responseError) {
|
||||
// Render error if page download failed
|
||||
tableContent = (
|
||||
<pre
|
||||
<div
|
||||
css={css`
|
||||
margin-top: ${theme.sizeUnit * 4}px;
|
||||
`}
|
||||
>
|
||||
{responseError}
|
||||
</pre>
|
||||
<Alert
|
||||
type="error"
|
||||
showIcon
|
||||
message={t('Failed to load drill-to-detail rows')}
|
||||
description={responseError}
|
||||
/>
|
||||
</div>
|
||||
);
|
||||
} else if (bootstrapping) {
|
||||
// Render loading if first page hasn't loaded
|
||||
|
||||
@@ -50,6 +50,7 @@ const DISABLED_REASONS = {
|
||||
DATABASE: t(
|
||||
'Drill to detail is disabled for this database. Change the database settings to enable it.',
|
||||
),
|
||||
DATASOURCE: t('Drill to detail is not available for this datasource type.'),
|
||||
NO_AGGREGATIONS: t(
|
||||
'Drill to detail is disabled because this chart does not group data by dimension value.',
|
||||
),
|
||||
@@ -116,6 +117,17 @@ export const useDrillDetailMenuItems = ({
|
||||
datasources[formData.datasource]?.database?.disable_drill_to_detail,
|
||||
);
|
||||
|
||||
// Capability flag on the datasource itself. Datasources that don't model
|
||||
// raw rows (e.g. semantic views) opt out via ``supports_drill_to_detail``
|
||||
// in the explore data payload.
|
||||
const datasourceSupportsDrillToDetail = useSelector<
|
||||
RootState,
|
||||
boolean | undefined
|
||||
>(
|
||||
({ datasources }) =>
|
||||
datasources[formData.datasource]?.supports_drill_to_detail,
|
||||
);
|
||||
|
||||
const openModal = useCallback(
|
||||
(filters: BinaryQueryObjectFilterClause[], event: MouseEvent) => {
|
||||
onClick(event);
|
||||
@@ -158,7 +170,10 @@ export const useDrillDetailMenuItems = ({
|
||||
|
||||
let drillDisabled;
|
||||
let drillByDisabled;
|
||||
if (drillToDetailDisabled) {
|
||||
if (datasourceSupportsDrillToDetail === false) {
|
||||
drillDisabled = DISABLED_REASONS.DATASOURCE;
|
||||
drillByDisabled = DISABLED_REASONS.DATASOURCE;
|
||||
} else if (drillToDetailDisabled) {
|
||||
drillDisabled = DISABLED_REASONS.DATABASE;
|
||||
drillByDisabled = DISABLED_REASONS.DATABASE;
|
||||
} else if (handlesDimensionContextMenu) {
|
||||
|
||||
@@ -444,3 +444,45 @@ test('context menu renders <NULL> for null dimension values', async () => {
|
||||
await expectDrillToDetailByEnabled();
|
||||
await expectDrillToDetailByDimension(filterNull);
|
||||
});
|
||||
|
||||
const buildStateWithUnsupportedDatasource = () => {
|
||||
const baseState = getMockStoreWithNativeFilters().getState();
|
||||
const datasourceKey = defaultFormData.datasource as string;
|
||||
return {
|
||||
...baseState,
|
||||
datasources: {
|
||||
...baseState.datasources,
|
||||
[datasourceKey]: {
|
||||
...baseState.datasources[datasourceKey],
|
||||
supports_drill_to_detail: false,
|
||||
},
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
test('dropdown menu when datasource opts out via supports_drill_to_detail=false', async () => {
|
||||
cleanup();
|
||||
render(<MockRenderChart formData={defaultFormData} />, {
|
||||
useRouter: true,
|
||||
useRedux: true,
|
||||
initialState: buildStateWithUnsupportedDatasource(),
|
||||
});
|
||||
|
||||
await expectDrillToDetailDisabled(
|
||||
'Drill to detail is not available for this datasource type.',
|
||||
);
|
||||
await expectNoDrillToDetailBy();
|
||||
});
|
||||
|
||||
test('context menu when datasource opts out via supports_drill_to_detail=false', async () => {
|
||||
cleanup();
|
||||
render(<MockRenderChart formData={defaultFormData} isContextMenu />, {
|
||||
useRouter: true,
|
||||
useRedux: true,
|
||||
initialState: buildStateWithUnsupportedDatasource(),
|
||||
});
|
||||
|
||||
const message = 'Drill to detail is not available for this datasource type.';
|
||||
await expectDrillToDetailDisabled(message);
|
||||
await expectDrillToDetailByDisabled(message);
|
||||
});
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
/**
|
||||
* Licensed to the Apache Software Foundation (ASF) under one
|
||||
* or more contributor license agreements. See the NOTICE file
|
||||
* distributed with this work for additional information
|
||||
* regarding copyright ownership. The ASF licenses this file
|
||||
* to you under the Apache License, Version 2.0 (the
|
||||
* "License"); you may not use this file except in compliance
|
||||
* with the License. You may obtain a copy of the License at
|
||||
*
|
||||
* http://www.apache.org/licenses/LICENSE-2.0
|
||||
*
|
||||
* Unless required by applicable law or agreed to in writing,
|
||||
* software distributed under the License is distributed on an
|
||||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
* KIND, either express or implied. See the License for the
|
||||
* specific language governing permissions and limitations
|
||||
* under the License.
|
||||
*/
|
||||
import { render, screen, fireEvent } from 'spec/helpers/testing-library';
|
||||
import DeleteComponentButton from './DeleteComponentButton';
|
||||
|
||||
test('exposes an accessible name without rendering visible label text', () => {
|
||||
render(<DeleteComponentButton onDelete={jest.fn()} />);
|
||||
|
||||
expect(
|
||||
screen.getByRole('button', { name: 'Delete component' }),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.queryByText('Delete component')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('calls onDelete when clicked', () => {
|
||||
const onDelete = jest.fn();
|
||||
render(<DeleteComponentButton onDelete={onDelete} />);
|
||||
|
||||
fireEvent.click(screen.getByRole('button', { name: 'Delete component' }));
|
||||
|
||||
expect(onDelete).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
@@ -18,6 +18,7 @@
|
||||
*/
|
||||
import { MouseEventHandler, FC } from 'react';
|
||||
|
||||
import { t } from '@apache-superset/core/translation';
|
||||
import { Icons } from '@superset-ui/core/components/Icons';
|
||||
import type { IconType } from '@superset-ui/core/components/Icons/types';
|
||||
import IconButton from './IconButton';
|
||||
@@ -33,6 +34,8 @@ const DeleteComponentButton: FC<DeleteComponentButtonProps> = ({
|
||||
}) => (
|
||||
<IconButton
|
||||
onClick={onDelete}
|
||||
label={t('Delete component')}
|
||||
hideVisibleLabel
|
||||
icon={<Icons.DeleteOutlined iconSize={iconSize ?? 'l'} />}
|
||||
/>
|
||||
);
|
||||
|
||||
@@ -73,3 +73,17 @@ test('renders the provided label', () => {
|
||||
|
||||
expect(screen.getByText('My Label')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('hideVisibleLabel suppresses visible text but keeps the accessible name', () => {
|
||||
render(
|
||||
<IconButton
|
||||
icon={icon}
|
||||
onClick={jest.fn()}
|
||||
label="My Label"
|
||||
hideVisibleLabel
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.queryByText('My Label')).not.toBeInTheDocument();
|
||||
expect(screen.getByRole('button', { name: 'My Label' })).toBeInTheDocument();
|
||||
});
|
||||
|
||||
@@ -22,6 +22,7 @@ import { styled, SupersetTheme } from '@apache-superset/core/theme';
|
||||
interface IconButtonProps extends HTMLAttributes<HTMLButtonElement> {
|
||||
icon: JSX.Element;
|
||||
label?: string;
|
||||
hideVisibleLabel?: boolean;
|
||||
onClick: MouseEventHandler<HTMLButtonElement>;
|
||||
disabled?: boolean;
|
||||
'data-test'?: string;
|
||||
@@ -63,6 +64,7 @@ const IconButton = forwardRef<HTMLButtonElement, IconButtonProps>(
|
||||
{
|
||||
icon,
|
||||
label,
|
||||
hideVisibleLabel,
|
||||
onClick,
|
||||
onKeyDown,
|
||||
disabled,
|
||||
@@ -75,6 +77,7 @@ const IconButton = forwardRef<HTMLButtonElement, IconButtonProps>(
|
||||
{...rest}
|
||||
ref={ref}
|
||||
type="button"
|
||||
aria-label={label}
|
||||
isDisabled={disabled}
|
||||
aria-disabled={disabled}
|
||||
data-test={dataTest}
|
||||
@@ -91,7 +94,7 @@ const IconButton = forwardRef<HTMLButtonElement, IconButtonProps>(
|
||||
}}
|
||||
>
|
||||
{icon}
|
||||
{label && <StyledSpan>{label}</StyledSpan>}
|
||||
{label && !hideVisibleLabel && <StyledSpan>{label}</StyledSpan>}
|
||||
</StyledButton>
|
||||
),
|
||||
);
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
* under the License.
|
||||
*/
|
||||
import React from 'react';
|
||||
import { fireEvent, render } from 'spec/helpers/testing-library';
|
||||
import { fireEvent, render, screen } from 'spec/helpers/testing-library';
|
||||
|
||||
import BackgroundStyleDropdown from 'src/dashboard/components/menu/BackgroundStyleDropdown';
|
||||
import IconButton from 'src/dashboard/components/IconButton';
|
||||
@@ -200,6 +200,15 @@ test('should call deleteComponent when deleted', () => {
|
||||
expect(deleteComponent).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test('settings IconButton exposes an accessible name without visible label text', () => {
|
||||
setup({ component: columnWithoutChildren, editMode: true });
|
||||
|
||||
expect(
|
||||
screen.getByRole('button', { name: 'Column settings' }),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.queryByText('Column settings')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('should pass its own width as availableColumnCount to children', () => {
|
||||
const { getByTestId } = setup();
|
||||
expect(getByTestId('mock-dashboard-component')).toHaveTextContent(
|
||||
|
||||
@@ -247,6 +247,8 @@ const Column = (props: ColumnProps) => {
|
||||
/>
|
||||
<IconButton
|
||||
onClick={() => handleChangeFocus(true)}
|
||||
label={t('Column settings')}
|
||||
hideVisibleLabel
|
||||
icon={<Icons.SettingOutlined iconSize="m" />}
|
||||
/>
|
||||
</HoverMenu>
|
||||
|
||||
@@ -145,7 +145,9 @@ describe('Header', () => {
|
||||
const deleteComponent = jest.fn();
|
||||
setup({ editMode: true, deleteComponent });
|
||||
|
||||
const trashButton = screen.getByRole('button', { name: 'delete' });
|
||||
const trashButton = screen.getByRole('button', {
|
||||
name: 'Delete component',
|
||||
});
|
||||
fireEvent.click(trashButton);
|
||||
|
||||
expect(deleteComponent).toHaveBeenCalledTimes(1);
|
||||
|
||||
@@ -240,6 +240,15 @@ test('should call deleteComponent when deleted', () => {
|
||||
expect(deleteComponent).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test('settings IconButton exposes an accessible name without visible label text', () => {
|
||||
setup({ component: rowWithoutChildren, editMode: true });
|
||||
|
||||
expect(
|
||||
screen.getByRole('button', { name: 'Row settings' }),
|
||||
).toBeInTheDocument();
|
||||
expect(screen.queryByText('Row settings')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('should pass appropriate availableColumnCount to children', () => {
|
||||
const { getByTestId } = setup();
|
||||
expect(getByTestId('mock-dashboard-component')).toHaveTextContent(
|
||||
|
||||
@@ -293,6 +293,8 @@ const Row = memo((props: RowProps) => {
|
||||
<DeleteComponentButton onDelete={handleDeleteComponent} />
|
||||
<IconButton
|
||||
onClick={() => handleChangeFocus(true)}
|
||||
label={t('Row settings')}
|
||||
hideVisibleLabel
|
||||
icon={<Icons.SettingOutlined iconSize="l" />}
|
||||
/>
|
||||
</HoverMenu>
|
||||
|
||||
@@ -232,6 +232,10 @@ export type Datasource = Dataset & {
|
||||
// Populated by the dashboard datasets API alongside ``type``; declared here
|
||||
// so callers can rely on structural typing instead of casting.
|
||||
datasource_type?: DatasourceType;
|
||||
/** False when the datasource can't return row samples (e.g. semantic views). */
|
||||
supports_samples?: boolean;
|
||||
/** False when the datasource can't answer drill-to-detail requests. */
|
||||
supports_drill_to_detail?: boolean;
|
||||
};
|
||||
export type DatasourcesState = {
|
||||
[key: string]: Datasource;
|
||||
|
||||
@@ -241,25 +241,44 @@ export const DataTablesPane = ({
|
||||
}
|
||||
}, [resultsTabFallback]);
|
||||
|
||||
// Hide the Samples tab for datasources that don't expose raw rows
|
||||
// (e.g. semantic views). The check is intentionally ``=== false`` so that
|
||||
// datasources from older backends that don't send the flag still show the
|
||||
// tab and preserve current behavior.
|
||||
const showSamplesTab = datasource?.supports_samples !== false;
|
||||
|
||||
// If the datasource swaps to one that doesn't support samples while the
|
||||
// Samples tab is active (e.g. the user picks a semantic view), the tab
|
||||
// disappears from ``tabItems`` and ``activeTabKey`` is orphaned. Fall back
|
||||
// to Results so the panel keeps rendering content.
|
||||
useEffect(() => {
|
||||
if (!showSamplesTab && activeTabKey === ResultTypes.Samples) {
|
||||
setActiveTabKey(ResultTypes.Results);
|
||||
}
|
||||
}, [showSamplesTab, activeTabKey]);
|
||||
const tabItems = [
|
||||
...queryResultsPanes,
|
||||
{
|
||||
key: ResultTypes.Samples,
|
||||
label: t('Samples'),
|
||||
children: (
|
||||
<StyledDiv>
|
||||
<SamplesPane
|
||||
datasource={datasource}
|
||||
queryFormData={queryFormData}
|
||||
queryForce={queryForce}
|
||||
isRequest={isRequest.samples}
|
||||
setForceQuery={setForceQuery}
|
||||
isVisible={ResultTypes.Samples === activeTabKey}
|
||||
canDownload={canDownload}
|
||||
/>
|
||||
</StyledDiv>
|
||||
),
|
||||
},
|
||||
...(showSamplesTab
|
||||
? [
|
||||
{
|
||||
key: ResultTypes.Samples,
|
||||
label: t('Samples'),
|
||||
children: (
|
||||
<StyledDiv>
|
||||
<SamplesPane
|
||||
datasource={datasource}
|
||||
queryFormData={queryFormData}
|
||||
queryForce={queryForce}
|
||||
isRequest={isRequest.samples}
|
||||
setForceQuery={setForceQuery}
|
||||
isVisible={ResultTypes.Samples === activeTabKey}
|
||||
canDownload={canDownload}
|
||||
/>
|
||||
</StyledDiv>
|
||||
),
|
||||
},
|
||||
]
|
||||
: []),
|
||||
];
|
||||
|
||||
return (
|
||||
|
||||
@@ -21,6 +21,7 @@ import { t } from '@apache-superset/core/translation';
|
||||
import { ensureIsArray } from '@superset-ui/core';
|
||||
import { datasetLabelLower } from 'src/features/semanticLayers/label';
|
||||
import { styled } from '@apache-superset/core/theme';
|
||||
import { Alert } from '@apache-superset/core/components';
|
||||
import { EmptyState, Loading } from '@superset-ui/core/components';
|
||||
import { GenericDataType } from '@apache-superset/core/common';
|
||||
import { GridTable } from 'src/components/GridTable';
|
||||
@@ -35,7 +36,7 @@ import {
|
||||
import { TableControls, ROW_LIMIT_OPTIONS } from './DataTableControls';
|
||||
import { SamplesPaneProps } from '../types';
|
||||
|
||||
const Error = styled.pre`
|
||||
const ErrorAlertWrapper = styled.div`
|
||||
margin-top: ${({ theme }) => `${theme.sizeUnit * 4}px`};
|
||||
`;
|
||||
|
||||
@@ -155,7 +156,14 @@ export const SamplesPane = ({
|
||||
rowLimitOptions={ROW_LIMIT_OPTIONS}
|
||||
onRowLimitChange={handleRowLimitChange}
|
||||
/>
|
||||
<Error>{responseError}</Error>
|
||||
<ErrorAlertWrapper>
|
||||
<Alert
|
||||
type="error"
|
||||
showIcon
|
||||
message={t('Failed to load samples')}
|
||||
description={responseError}
|
||||
/>
|
||||
</ErrorAlertWrapper>
|
||||
</>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -27,13 +27,14 @@ import {
|
||||
QueryData,
|
||||
} from '@superset-ui/core';
|
||||
import { styled } from '@apache-superset/core/theme';
|
||||
import { Alert } from '@apache-superset/core/components';
|
||||
import { EmptyState, Loading } from '@superset-ui/core/components';
|
||||
import { getChartDataRequest } from 'src/components/Chart/chartAction';
|
||||
import { ResultsPaneProps, QueryResultInterface } from '../types';
|
||||
import { SingleQueryResultPane } from './SingleQueryResultPane';
|
||||
import { TableControls, ROW_LIMIT_OPTIONS } from './DataTableControls';
|
||||
|
||||
const Error = styled.pre`
|
||||
const ErrorAlertWrapper = styled.div`
|
||||
margin-top: ${({ theme }) => `${theme.sizeUnit * 4}px`};
|
||||
`;
|
||||
|
||||
@@ -199,7 +200,14 @@ export const useResultsPane = ({
|
||||
isLoading={false}
|
||||
canDownload={canDownload}
|
||||
/>
|
||||
<Error>{responseError}</Error>
|
||||
<ErrorAlertWrapper>
|
||||
<Alert
|
||||
type="error"
|
||||
showIcon
|
||||
message={t('Failed to load results')}
|
||||
description={responseError}
|
||||
/>
|
||||
</ErrorAlertWrapper>
|
||||
</>
|
||||
);
|
||||
return Array(queryCount).fill(err);
|
||||
|
||||
@@ -19,7 +19,12 @@
|
||||
import fetchMock from 'fetch-mock';
|
||||
import { FeatureFlag } from '@superset-ui/core';
|
||||
import * as copyUtils from 'src/utils/copy';
|
||||
import { render, screen, userEvent } from 'spec/helpers/testing-library';
|
||||
import {
|
||||
render,
|
||||
screen,
|
||||
userEvent,
|
||||
waitFor,
|
||||
} from 'spec/helpers/testing-library';
|
||||
import { setupAGGridModules } from '@superset-ui/core/components/ThemedAgGridReact';
|
||||
import { setItem, LocalStorageKeys } from 'src/utils/localStorageHelpers';
|
||||
import { DataTablesPane } from '..';
|
||||
@@ -89,6 +94,48 @@ describe('DataTablesPane', () => {
|
||||
expect(await screen.findByLabelText('Collapse data panel')).toBeVisible();
|
||||
});
|
||||
|
||||
test('Hides Samples tab when datasource opts out via supports_samples=false', async () => {
|
||||
const props = createDataTablesPaneProps(0);
|
||||
const propsWithoutSamples = {
|
||||
...props,
|
||||
datasource: { ...props.datasource, supports_samples: false },
|
||||
};
|
||||
render(<DataTablesPane {...propsWithoutSamples} />, { useRedux: true });
|
||||
expect(await screen.findByText('Results')).toBeVisible();
|
||||
expect(screen.queryByText('Samples')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('Falls back to Results when active Samples tab disappears mid-session', async () => {
|
||||
// Regression for codeant Major finding on PR #41509: a datasource swap
|
||||
// that hides the Samples tab while it was the active tab used to leave
|
||||
// ``activeTabKey === 'samples'`` orphaned, rendering a blank panel.
|
||||
const props = createDataTablesPaneProps(0);
|
||||
const { rerender } = render(<DataTablesPane {...props} />, {
|
||||
useRedux: true,
|
||||
});
|
||||
|
||||
// Open the panel and pick the Samples tab.
|
||||
userEvent.click(screen.getByLabelText('Expand data panel'));
|
||||
userEvent.click(await screen.findByText('Samples'));
|
||||
expect(await screen.findByLabelText('Collapse data panel')).toBeVisible();
|
||||
|
||||
// Swap to a datasource that doesn't support samples (e.g. a semantic
|
||||
// view). The Samples tab should disappear and the panel should land on
|
||||
// Results with content still rendered.
|
||||
rerender(
|
||||
<DataTablesPane
|
||||
{...props}
|
||||
datasource={{ ...props.datasource, supports_samples: false }}
|
||||
/>,
|
||||
);
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByText('Samples')).not.toBeInTheDocument();
|
||||
});
|
||||
expect(screen.getByText('Results')).toBeVisible();
|
||||
// Panel stays expanded and renders Results content rather than going blank.
|
||||
expect(screen.getByLabelText('Collapse data panel')).toBeVisible();
|
||||
});
|
||||
|
||||
test('Should copy data table content correctly', async () => {
|
||||
fetchMock.post(
|
||||
'glob:*/api/v1/chart/data?form_data=%7B%22slice_id%22%3A456%7D',
|
||||
|
||||
@@ -84,10 +84,14 @@ describe('SamplesPane', () => {
|
||||
const props = createSamplesPaneProps({
|
||||
datasourceId: 36,
|
||||
});
|
||||
const { findByText } = render(<SamplesPane {...props} />, {
|
||||
const { findByText, findByRole } = render(<SamplesPane {...props} />, {
|
||||
useRedux: true,
|
||||
});
|
||||
|
||||
// The error is now rendered inside an Alert component, with a clear
|
||||
// headline message and the raw error text as the description.
|
||||
expect(await findByRole('alert')).toBeVisible();
|
||||
expect(await findByText('Failed to load samples')).toBeVisible();
|
||||
expect(await findByText('Error: Bad request')).toBeVisible();
|
||||
});
|
||||
|
||||
|
||||
@@ -74,6 +74,18 @@ export type Datasource = Dataset & {
|
||||
schema?: string;
|
||||
is_sqllab_view?: boolean;
|
||||
extra?: string | object;
|
||||
/**
|
||||
* False when the datasource (e.g. a semantic view) doesn't model raw rows
|
||||
* and therefore can't return a row sample. Defaults to true on the server
|
||||
* side; missing here means the explore UI keeps current behavior.
|
||||
*/
|
||||
supports_samples?: boolean;
|
||||
/**
|
||||
* False when the datasource doesn't model raw rows and therefore can't
|
||||
* answer a drill-to-detail query. Tracked separately from
|
||||
* ``supports_samples`` so the two capabilities can diverge.
|
||||
*/
|
||||
supports_drill_to_detail?: boolean;
|
||||
};
|
||||
|
||||
export interface ExplorePageInitialData {
|
||||
|
||||
@@ -1052,6 +1052,59 @@ describe('active tab highlighting (regression #36403)', () => {
|
||||
expect(getMenuItemByText('Дашборды')).toHaveClass('ant-menu-item-selected');
|
||||
});
|
||||
|
||||
test.each([
|
||||
['/tablemodelview/list/', 'the legacy FAB dataset list route'],
|
||||
['/dataset/add/', 'the modern React dataset create route'],
|
||||
['/dataset/42', 'a dataset detail route'],
|
||||
])(
|
||||
'highlights the Datasets tab on %s (%s) — regression #42467',
|
||||
async (route, _label) => {
|
||||
// ``/tablemodelview/list/`` is the pre-existing legacy path (kept as
|
||||
// a coverage anchor so future prefix-matching changes cannot silently
|
||||
// regress it); the ``/dataset/*`` routes are the modern React ones
|
||||
// added by #42467.
|
||||
useSelectorMock.mockReturnValue({ roles: user.roles });
|
||||
window.history.pushState({}, '', route);
|
||||
|
||||
render(<Menu {...mockedProps} />, {
|
||||
useRedux: true,
|
||||
useQueryParams: true,
|
||||
useRouter: true,
|
||||
useTheme: true,
|
||||
});
|
||||
|
||||
// Datasets is a child under the Sources submenu — expand it first so
|
||||
// the item is in the DOM (same pattern as the existing "render the top
|
||||
// navbar child menu items" test).
|
||||
const sources = await screen.findByText('Sources');
|
||||
userEvent.hover(sources);
|
||||
|
||||
const datasets = await screen.findByText('Datasets');
|
||||
expect(datasets.closest('li')).toHaveClass('ant-menu-item-selected');
|
||||
},
|
||||
);
|
||||
|
||||
test('does not highlight the Datasets tab on lookalike prefixes (e.g. /datasetXyz)', async () => {
|
||||
// The active-tab matcher must use a boundary-aware startsWith so that
|
||||
// an unrelated future route beginning with ``/dataset`` (e.g. a
|
||||
// hypothetical ``/datasetXyz``) does not falsely trigger the highlight.
|
||||
useSelectorMock.mockReturnValue({ roles: user.roles });
|
||||
window.history.pushState({}, '', '/datasetXyz');
|
||||
|
||||
render(<Menu {...mockedProps} />, {
|
||||
useRedux: true,
|
||||
useQueryParams: true,
|
||||
useRouter: true,
|
||||
useTheme: true,
|
||||
});
|
||||
|
||||
const sources = await screen.findByText('Sources');
|
||||
userEvent.hover(sources);
|
||||
|
||||
const datasets = await screen.findByText('Datasets');
|
||||
expect(datasets.closest('li')).not.toHaveClass('ant-menu-item-selected');
|
||||
});
|
||||
|
||||
test('highlights the active SQL tab when the label is localized', async () => {
|
||||
// The SQL Lab top-level entry is a FAB category: its stable `name` is
|
||||
// "SQL Lab" while its label ("SQL") is localized.
|
||||
|
||||
@@ -212,6 +212,12 @@ export function Menu({
|
||||
Dashboard = '/dashboard',
|
||||
Chart = '/chart',
|
||||
Datasets = '/tablemodelview',
|
||||
// The legacy FAB dataset list still lives at ``/tablemodelview/list/``,
|
||||
// but the modern React-managed dataset add + detail routes are under
|
||||
// ``/dataset/*`` (``/dataset/add/``, ``/dataset/:datasetId``). Both
|
||||
// prefixes must map to the Datasets tab so the top-nav highlight
|
||||
// survives navigation into the create/edit flow. See #42467.
|
||||
Dataset = '/dataset',
|
||||
SqlLab = '/sqllab',
|
||||
SavedQueries = '/savedqueryview',
|
||||
}
|
||||
@@ -238,7 +244,9 @@ export function Menu({
|
||||
case path.startsWith(Paths.Chart) || path.startsWith(Paths.Explore):
|
||||
setActiveTabs([MenuKeys.Charts]);
|
||||
break;
|
||||
case path.startsWith(Paths.Datasets):
|
||||
case path.startsWith(Paths.Datasets) ||
|
||||
path === Paths.Dataset ||
|
||||
path.startsWith(`${Paths.Dataset}/`):
|
||||
setActiveTabs([MenuKeys.Datasets]);
|
||||
break;
|
||||
case path.startsWith(Paths.SqlLab) || path.startsWith(Paths.SavedQueries):
|
||||
|
||||
@@ -17,7 +17,7 @@
|
||||
import logging
|
||||
from typing import Any, Optional
|
||||
|
||||
from croniter import croniter
|
||||
from croniter import croniter, CroniterBadDateError
|
||||
from flask import current_app as app
|
||||
from flask_babel import gettext as _
|
||||
from marshmallow import ValidationError
|
||||
@@ -29,6 +29,7 @@ from superset.commands.report.exceptions import (
|
||||
ChartNotSavedValidationError,
|
||||
DashboardNotFoundValidationError,
|
||||
DashboardNotSavedValidationError,
|
||||
ReportScheduleCrontabNotValidError,
|
||||
ReportScheduleEitherChartOrDashboardError,
|
||||
ReportScheduleForbiddenError,
|
||||
ReportScheduleFrequencyNotAllowed,
|
||||
@@ -288,13 +289,18 @@ class BaseReportScheduleCommand(BaseCommand):
|
||||
return
|
||||
|
||||
iterations = 60 if minimum_interval <= 3660 else 24
|
||||
schedule = croniter(cron_schedule)
|
||||
current_exec = next(schedule)
|
||||
try:
|
||||
schedule = croniter(cron_schedule)
|
||||
current_exec = next(schedule)
|
||||
|
||||
for _i in range(iterations):
|
||||
next_exec = next(schedule)
|
||||
diff, current_exec = next_exec - current_exec, next_exec
|
||||
if int(diff) < minimum_interval:
|
||||
raise ReportScheduleFrequencyNotAllowed(
|
||||
report_type=report_type, minimum_interval=minimum_interval
|
||||
)
|
||||
for _i in range(iterations):
|
||||
next_exec = next(schedule)
|
||||
diff, current_exec = next_exec - current_exec, next_exec
|
||||
if int(diff) < minimum_interval:
|
||||
raise ReportScheduleFrequencyNotAllowed(
|
||||
report_type=report_type, minimum_interval=minimum_interval
|
||||
)
|
||||
except CroniterBadDateError as ex:
|
||||
raise ReportScheduleCrontabNotValidError(
|
||||
cron_schedule=cron_schedule
|
||||
) from ex
|
||||
|
||||
@@ -133,6 +133,23 @@ class ReportScheduleFrequencyNotAllowed(ValidationError): # noqa: N818
|
||||
)
|
||||
|
||||
|
||||
class ReportScheduleCrontabNotValidError(ValidationError): # noqa: N818
|
||||
"""
|
||||
Marshmallow validation error for a crontab that is syntactically valid
|
||||
but never matches a real calendar date (e.g. February 30th)
|
||||
"""
|
||||
|
||||
def __init__(self, cron_schedule: str = "") -> None:
|
||||
super().__init__(
|
||||
_(
|
||||
"Invalid crontab schedule: %(cron_schedule)s never matches"
|
||||
" a valid date",
|
||||
cron_schedule=cron_schedule,
|
||||
),
|
||||
field_name="crontab",
|
||||
)
|
||||
|
||||
|
||||
class ChartNotSavedValidationError(ValidationError):
|
||||
"""
|
||||
Marshmallow validation error for charts that haven't been saved yet
|
||||
|
||||
@@ -235,6 +235,15 @@ def _get_drill_detail(
|
||||
# todo(yongjie): Remove this function,
|
||||
# when determining whether samples should be applied to the time filter.
|
||||
datasource = _get_datasource(query_context, query_obj)
|
||||
# Refuse for datasource types that don't model raw rows (e.g. semantic
|
||||
# views). Mirrors the ``supports_samples`` gate on the ``/samples``
|
||||
# endpoint so drill-detail is hard-blocked on the backend, not just
|
||||
# hidden in the frontend menu. Defaults to ``True`` for any datasource
|
||||
# class that doesn't explicitly opt out.
|
||||
if not getattr(datasource, "supports_drill_to_detail", True):
|
||||
raise QueryObjectValidationError(
|
||||
_("Drill to detail is not available for this datasource type.")
|
||||
)
|
||||
query_obj = copy.copy(query_obj)
|
||||
query_obj.is_timeseries = False
|
||||
query_obj.metrics = None
|
||||
|
||||
@@ -193,6 +193,16 @@ class BaseDatasource(
|
||||
# Only some datasources support Row Level Security
|
||||
is_rls_supported: bool = False
|
||||
|
||||
# Datasources that can return raw row samples (anything backed by a SQL
|
||||
# table can; semantic-layer abstractions cannot, since they only expose
|
||||
# pre-defined metrics and dimensions).
|
||||
supports_samples: bool = True
|
||||
|
||||
# Datasources that can answer "drill to detail" requests — i.e. fetch the
|
||||
# raw rows underlying a chart cell. Conceptually similar to ``samples``
|
||||
# but kept as a separate capability so the two can diverge.
|
||||
supports_drill_to_detail: bool = True
|
||||
|
||||
@property
|
||||
def name(self) -> str:
|
||||
# can be a Column or a property pointing to one
|
||||
@@ -486,6 +496,8 @@ class BaseDatasource(
|
||||
"order_by_choices": self.order_by_choices,
|
||||
"verbose_map": self.verbose_map,
|
||||
"select_star": self.select_star,
|
||||
"supports_samples": self.supports_samples,
|
||||
"supports_drill_to_detail": self.supports_drill_to_detail,
|
||||
}
|
||||
|
||||
def data_for_slices( # pylint: disable=too-many-locals # noqa: C901
|
||||
|
||||
@@ -125,23 +125,17 @@ class Db2EngineSpec(BaseEngineSpec):
|
||||
"""
|
||||
Get comment of table from a given schema
|
||||
|
||||
Ibm Db2 return comments as tuples, so we need to get the first element
|
||||
|
||||
:param inspector: SqlAlchemy Inspector instance
|
||||
:param table: Table instance
|
||||
:return: comment of table
|
||||
"""
|
||||
comment = None
|
||||
try:
|
||||
table_comment = inspector.get_table_comment(table.table, table.schema)
|
||||
comment = table_comment.get("text")
|
||||
return comment[0]
|
||||
except IndexError:
|
||||
return comment
|
||||
return table_comment.get("text")
|
||||
except Exception as ex: # pylint: disable=broad-except
|
||||
logger.error("Unexpected error while fetching table comment", exc_info=True)
|
||||
logger.exception(ex)
|
||||
return comment
|
||||
return None
|
||||
|
||||
@classmethod
|
||||
def get_prequeries(
|
||||
|
||||
@@ -821,6 +821,32 @@ WHERE datistemplate = false;
|
||||
)
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def get_schema_names(cls, inspector: Inspector) -> set[str]:
|
||||
"""
|
||||
Return all schema names, excluding the ``pg_``-prefixed Postgres
|
||||
system schemas (e.g. ``pg_catalog``, ``pg_toast``).
|
||||
|
||||
SQLAlchemy's Postgres dialect filters out system schemas with the
|
||||
query ``nspname NOT LIKE 'pg_%'``. Since ``_`` is a single-character
|
||||
wildcard in SQL ``LIKE`` patterns, this unintentionally excludes any
|
||||
user-defined schema that merely starts with ``pg`` followed by any
|
||||
other character (e.g. ``pgsql``, ``pgstats``), not only the
|
||||
``pg_``-prefixed system schemas. Matching on the literal ``pg_``
|
||||
prefix instead keeps those user-defined schemas.
|
||||
|
||||
TODO: drop this override once sqlalchemy/sqlalchemy#13471 is merged
|
||||
and released, and SQLAlchemy is bumped past that version.
|
||||
"""
|
||||
with inspector.engine.connect() as conn:
|
||||
return {
|
||||
name
|
||||
for (name,) in conn.execute(
|
||||
text("SELECT nspname FROM pg_namespace ORDER BY nspname")
|
||||
)
|
||||
if not name.startswith("pg_")
|
||||
}
|
||||
|
||||
@classmethod
|
||||
def get_table_names(
|
||||
cls, database: Database, inspector: PGInspector, schema: str | None
|
||||
|
||||
@@ -157,7 +157,7 @@ Add this to your Claude Desktop config file:
|
||||
|
||||
**macOS**: `~/Library/Application Support/Claude/claude_desktop_config.json`
|
||||
|
||||
Since claude desktop doesnt like non https mcp servers you can use this proxy:
|
||||
Since claude desktop doesn't like non https mcp servers you can use this proxy:
|
||||
```json
|
||||
{
|
||||
"mcpServers": {
|
||||
|
||||
@@ -56,10 +56,6 @@ SUPERSET_WEBSERVER_ADDRESS = "http://localhost:9001"
|
||||
WEBDRIVER_BASEURL = "http://localhost:9001/"
|
||||
WEBDRIVER_BASEURL_USER_FRIENDLY = WEBDRIVER_BASEURL
|
||||
|
||||
# MCP Service Host/Port
|
||||
MCP_SERVICE_HOST = "localhost"
|
||||
MCP_SERVICE_PORT = 5008
|
||||
|
||||
# Bug-report support contact surfaced by the generate_bug_report tool. Each
|
||||
# deployment should override this in superset_config.py to point users at the
|
||||
# right channel (e.g. an internal support address, a vendor support team).
|
||||
@@ -691,8 +687,6 @@ def get_mcp_config(app_config: dict[str, Any] | None = None) -> dict[str, Any]:
|
||||
"SUPERSET_WEBSERVER_ADDRESS": SUPERSET_WEBSERVER_ADDRESS,
|
||||
"WEBDRIVER_BASEURL": WEBDRIVER_BASEURL,
|
||||
"WEBDRIVER_BASEURL_USER_FRIENDLY": WEBDRIVER_BASEURL_USER_FRIENDLY,
|
||||
"MCP_SERVICE_HOST": MCP_SERVICE_HOST,
|
||||
"MCP_SERVICE_PORT": MCP_SERVICE_PORT,
|
||||
"MCP_DEBUG": MCP_DEBUG,
|
||||
"MCP_RBAC_ENABLED": MCP_RBAC_ENABLED,
|
||||
"MCP_DISABLED_TOOLS": set(MCP_DISABLED_TOOLS),
|
||||
|
||||
@@ -2054,8 +2054,8 @@ class ExploreMixin: # pylint: disable=too-many-public-methods
|
||||
offset, outer_to_dttm
|
||||
)
|
||||
|
||||
query_object_clone.inner_from_dttm = query_object_clone.from_dttm
|
||||
query_object_clone.inner_to_dttm = query_object_clone.to_dttm
|
||||
query_object_clone.inner_from_dttm = outer_from_dttm
|
||||
query_object_clone.inner_to_dttm = outer_to_dttm
|
||||
|
||||
x_axis_label = get_x_axis_label(query_object.columns)
|
||||
query_object_clone.granularity = (
|
||||
|
||||
@@ -200,6 +200,12 @@ class SemanticView(AuditMixinNullable, Model):
|
||||
|
||||
__tablename__ = "semantic_views"
|
||||
|
||||
# Semantic views expose pre-defined metrics and dimensions, not raw rows,
|
||||
# so neither the "Samples" tab in Explore nor the "Drill to detail"
|
||||
# affordance from the chart 3-dots menu can return anything meaningful.
|
||||
supports_samples: bool = False
|
||||
supports_drill_to_detail: bool = False
|
||||
|
||||
# Use integer as the primary key for cross-database auto-increment
|
||||
# compatibility (sa.Identity() is not supported in MySQL or SQLite).
|
||||
# The uuid column is a secondary unique identifier used in URLs and perms.
|
||||
@@ -425,6 +431,8 @@ class SemanticView(AuditMixinNullable, Model):
|
||||
"sql": None,
|
||||
"select_star": None,
|
||||
"editors": [],
|
||||
"supports_samples": self.supports_samples,
|
||||
"supports_drill_to_detail": self.supports_drill_to_detail,
|
||||
"description": self.description,
|
||||
"table_name": self.name,
|
||||
"column_types": [
|
||||
|
||||
@@ -346,6 +346,11 @@ class ExplorableData(TypedDict, total=False):
|
||||
always_filter_main_dttm: bool
|
||||
normalize_columns: bool
|
||||
rls_filters: list[dict[str, Any]]
|
||||
# Set by datasources that cannot return raw row samples (e.g. semantic
|
||||
# views, which only expose pre-defined metrics and dimensions).
|
||||
supports_samples: bool
|
||||
# Set by datasources that cannot answer drill-to-detail requests.
|
||||
supports_drill_to_detail: bool
|
||||
|
||||
|
||||
VizData: TypeAlias = list[Any] | dict[Any, Any] | None
|
||||
|
||||
@@ -155,25 +155,39 @@ def get_executor( # noqa: C901
|
||||
if executor == ExecutorType.CURRENT_USER and current_user:
|
||||
return executor, current_user
|
||||
if executor == ExecutorType.CREATOR_EDITOR:
|
||||
if (user := model.created_by) and _is_editor(user.id):
|
||||
if (user := model.created_by) and user.is_active and _is_editor(user.id):
|
||||
return executor, user.username
|
||||
if executor == ExecutorType.CREATOR:
|
||||
if user := model.created_by:
|
||||
if (user := model.created_by) and user.is_active:
|
||||
return executor, user.username
|
||||
if executor == ExecutorType.MODIFIER_EDITOR:
|
||||
if (user := model.changed_by) and _is_editor(user.id):
|
||||
if (user := model.changed_by) and user.is_active and _is_editor(user.id):
|
||||
return executor, user.username
|
||||
if executor == ExecutorType.MODIFIER:
|
||||
if user := model.changed_by:
|
||||
if (user := model.changed_by) and user.is_active:
|
||||
return executor, user.username
|
||||
if executor == ExecutorType.EDITOR:
|
||||
# Priority: modifier → creator → direct user editor → indirect editor.
|
||||
if (modifier := model.changed_by) and _is_editor(modifier.id):
|
||||
# Inactive users are skipped at every step so that scheduling can
|
||||
# fall through to another active owner/editor instead of failing
|
||||
# outright (see: ExecutorNotFoundError only once no active
|
||||
# candidate remains).
|
||||
if (
|
||||
(modifier := model.changed_by)
|
||||
and modifier.is_active
|
||||
and _is_editor(modifier.id)
|
||||
):
|
||||
return executor, modifier.username
|
||||
if (creator := model.created_by) and _is_editor(creator.id):
|
||||
if (
|
||||
(creator := model.created_by)
|
||||
and creator.is_active
|
||||
and _is_editor(creator.id)
|
||||
):
|
||||
return executor, creator.username
|
||||
if editor_users:
|
||||
return executor, editor_users[0].username
|
||||
if active_editor_user := next(
|
||||
(user for user in editor_users if user.is_active), None
|
||||
):
|
||||
return executor, active_editor_user.username
|
||||
if indirect_editor := _get_indirect_editor_user(
|
||||
getattr(model, "editors", [])
|
||||
):
|
||||
|
||||
@@ -87,6 +87,29 @@ def resolve_screenshot_task_budget_seconds(
|
||||
return None
|
||||
|
||||
|
||||
# Fallback wall-clock budget, in seconds, for the entire tiled-screenshot
|
||||
# operation (element lookup plus all per-tile readiness/animation waits
|
||||
# combined), used when resolve_screenshot_task_budget_seconds() returns None
|
||||
# (no Celery task context -- e.g. synchronous thumbnail generation -- or no
|
||||
# usable task limit). The non-tiled readiness path treats None as "keep the
|
||||
# configured SCREENSHOT_LOAD_WAIT" because it makes exactly one bounded wait;
|
||||
# the tiled path cannot, because its per-tile waits accumulate: with N tiles,
|
||||
# an uncapped load_wait allows N * load_wait of total wall-clock time, so the
|
||||
# operation still needs one fixed total ceiling. Sized against the longest
|
||||
# Celery hard task_time_limit observed in production for report execution
|
||||
# (1740s), minus the same 300s cleanup margin the runtime derivation reserves
|
||||
# for combining tiles, building the PDF, and delivering the notification.
|
||||
TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS = 1440 # 1740s limit - 300s margin
|
||||
|
||||
|
||||
class ScreenshotTaskBudgetExceededError(RuntimeError):
|
||||
"""Raised when no safe task budget remains before screenshot capture."""
|
||||
|
||||
|
||||
class TiledScreenshotBudgetExceededError(ScreenshotTaskBudgetExceededError):
|
||||
"""Raised when the tiled-screenshot time budget runs out mid-capture."""
|
||||
|
||||
|
||||
try:
|
||||
from playwright.sync_api import TimeoutError as PlaywrightTimeout
|
||||
except ImportError:
|
||||
@@ -251,7 +274,7 @@ def combine_screenshot_tiles(screenshot_tiles: list[bytes]) -> bytes:
|
||||
return screenshot_tiles[0]
|
||||
|
||||
|
||||
def take_tiled_screenshot(
|
||||
def take_tiled_screenshot( # noqa: C901
|
||||
page: "Page",
|
||||
element_name: str,
|
||||
tile_height: int,
|
||||
@@ -274,6 +297,12 @@ def take_tiled_screenshot(
|
||||
|
||||
Returns:
|
||||
Combined screenshot bytes or None if failed
|
||||
|
||||
Raises:
|
||||
TiledScreenshotBudgetExceededError: If the total time budget for the
|
||||
tiled-screenshot operation runs out before every tile has been
|
||||
verifiably captured. Callers must treat this as a hard failure
|
||||
rather than fall back to an unchecked/partial screenshot.
|
||||
"""
|
||||
context_suffix = f" [{log_context}]" if log_context else ""
|
||||
# Set right before re-raising the per-tile readiness timeout below, and
|
||||
@@ -286,6 +315,15 @@ def take_tiled_screenshot(
|
||||
# match `except PlaywrightTimeout` and incorrectly propagate instead of
|
||||
# degrading to `None` like every other unexpected error in this function.
|
||||
readiness_timeout = False
|
||||
# Cap the whole tiled operation against the running Celery task's own
|
||||
# time limit, using the same runtime derivation as the non-tiled
|
||||
# readiness wait (#42253/#42427). Unlike that path, a None budget does
|
||||
# not mean "keep the configured timeout": per-tile waits accumulate, so
|
||||
# the operation falls back to a fixed total ceiling instead.
|
||||
wait_budget_seconds = resolve_screenshot_task_budget_seconds(log_context)
|
||||
if wait_budget_seconds is None:
|
||||
wait_budget_seconds = float(TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS)
|
||||
start_time = time.monotonic()
|
||||
try:
|
||||
# Get the target element
|
||||
element = page.locator(f".{element_name}")
|
||||
@@ -320,9 +358,44 @@ def take_tiled_screenshot(
|
||||
num_tiles = max(1, (dashboard_height + tile_height - 1) // tile_height)
|
||||
logger.info("Taking %s screenshot tiles", num_tiles)
|
||||
|
||||
screenshot_tiles = []
|
||||
screenshot_tiles: list[bytes] = []
|
||||
|
||||
def _raise_if_budget_exhausted(elapsed: float, remaining_budget: float) -> None:
|
||||
if remaining_budget > 0:
|
||||
return
|
||||
# A customer-side chart-loading issue (a slow/hung dashboard),
|
||||
# not a Superset system fault, so this is a WARNING rather
|
||||
# than an ERROR -- consistent with #38130/#38441, which
|
||||
# deliberately downgraded screenshot timeout logs the same way.
|
||||
logger.warning(
|
||||
"Tiled screenshot time budget exhausted on tile %s/%s: "
|
||||
"%s/%s tiles captured so far, %.1fs elapsed of a %.1fs "
|
||||
"budget. Aborting instead of capturing remaining tiles "
|
||||
"unchecked.%s",
|
||||
i + 1,
|
||||
num_tiles,
|
||||
len(screenshot_tiles),
|
||||
num_tiles,
|
||||
elapsed,
|
||||
wait_budget_seconds,
|
||||
context_suffix,
|
||||
)
|
||||
raise TiledScreenshotBudgetExceededError(
|
||||
f"Tiled screenshot budget of "
|
||||
f"{wait_budget_seconds:.1f}s exhausted "
|
||||
f"after {len(screenshot_tiles)}/{num_tiles} tiles"
|
||||
)
|
||||
|
||||
for i in range(num_tiles):
|
||||
# Check the time budget before starting this tile's readiness wait.
|
||||
# If it's already exhausted, we can no longer verify this (or any
|
||||
# later) tile is actually ready to capture -- fail loudly instead
|
||||
# of silently snapshotting a spinner or blank chart, or running
|
||||
# past the Celery task time limit and getting SIGKILLed.
|
||||
elapsed = time.monotonic() - start_time
|
||||
remaining_budget = wait_budget_seconds - elapsed
|
||||
_raise_if_budget_exhausted(elapsed, remaining_budget)
|
||||
|
||||
# Calculate scroll position to show this tile's content
|
||||
scroll_y = dashboard_top + (i * tile_height)
|
||||
|
||||
@@ -332,17 +405,31 @@ def take_tiled_screenshot(
|
||||
)
|
||||
# Wait for scroll to settle and content to load
|
||||
page.wait_for_timeout(SCROLL_SETTLE_TIMEOUT_MS)
|
||||
|
||||
# Recompute the remaining budget after the scroll-settle sleep --
|
||||
# which itself consumes real wall-clock time -- rather than
|
||||
# reusing the value from before it, so the readiness-check
|
||||
# timeout below is capped against a fresh number instead of a
|
||||
# stale one that would let each tile overrun the budget by up
|
||||
# to one settle interval.
|
||||
tile_wait_start = time.monotonic()
|
||||
elapsed = tile_wait_start - start_time
|
||||
remaining_budget = wait_budget_seconds - elapsed
|
||||
_raise_if_budget_exhausted(elapsed, remaining_budget)
|
||||
|
||||
# Wait for every chart holder visible in the current viewport to reach
|
||||
# a terminal state (rendered chart or error/empty state). Only check
|
||||
# a terminal state (rendered chart or error/empty state), capped at
|
||||
# whatever remains of the total time budget so a slow dashboard
|
||||
# degrades gracefully instead of exceeding it. Only check
|
||||
# viewport-visible chart holders to avoid blocking on virtualization
|
||||
# placeholders rendered for off-screen charts. A holder that hasn't
|
||||
# mounted anything yet does not satisfy this check -- unlike checking
|
||||
# for the absence of `.loading`, which passes vacuously in that case.
|
||||
tile_wait_start = time.monotonic()
|
||||
tile_load_wait = min(load_wait, remaining_budget)
|
||||
try:
|
||||
page.wait_for_function(
|
||||
CHART_HOLDERS_READY_JS,
|
||||
timeout=load_wait * 1000,
|
||||
timeout=tile_load_wait * 1000,
|
||||
)
|
||||
except PlaywrightTimeout:
|
||||
elapsed = time.monotonic() - tile_wait_start
|
||||
@@ -354,14 +441,21 @@ def take_tiled_screenshot(
|
||||
# made the same call for the other screenshot timeout paths.
|
||||
logger.warning(
|
||||
"Timed out after %.2fs waiting for %s chart container(s) to "
|
||||
"become ready on tile %s/%s (load_wait=%ss)%s; unready chart "
|
||||
"holders (chart id, state): %s. Aborting tiled screenshot "
|
||||
"rather than capturing a blank or partially-loaded tile.",
|
||||
"become ready on tile %s/%s (waited %.1fs of a %ss requested "
|
||||
"load_wait; %.1fs elapsed of a %.1fs total budget; %s/%s "
|
||||
"tiles captured so far)%s; unready chart holders (chart id, "
|
||||
"state): %s. Aborting tiled screenshot rather than capturing "
|
||||
"a blank or partially-loaded tile.",
|
||||
elapsed,
|
||||
len(unready_chart_holders),
|
||||
i + 1,
|
||||
num_tiles,
|
||||
tile_load_wait,
|
||||
load_wait,
|
||||
time.monotonic() - start_time,
|
||||
wait_budget_seconds,
|
||||
len(screenshot_tiles),
|
||||
num_tiles,
|
||||
context_suffix,
|
||||
unready_chart_holders,
|
||||
)
|
||||
@@ -377,12 +471,36 @@ def take_tiled_screenshot(
|
||||
load_wait,
|
||||
context_suffix,
|
||||
)
|
||||
readiness_wait_elapsed = time.monotonic() - tile_wait_start
|
||||
|
||||
# Wait for chart animations (e.g. ECharts) to finish after spinner clears.
|
||||
# The global animation wait before tiling only covers the first tile;
|
||||
# subsequent tiles need their own wait after data loads.
|
||||
# subsequent tiles need their own wait after data loads. Capped at
|
||||
# whatever remains of the budget; unlike the readiness wait above this
|
||||
# is cosmetic settling, not a readiness check, so we simply skip it
|
||||
# (rather than raise) once the budget runs out.
|
||||
animation_wait_elapsed = 0.0
|
||||
if animation_wait > 0:
|
||||
page.wait_for_timeout(animation_wait * 1000)
|
||||
elapsed = time.monotonic() - start_time
|
||||
remaining_budget = wait_budget_seconds - elapsed
|
||||
tile_animation_wait = max(0, min(animation_wait, remaining_budget))
|
||||
if tile_animation_wait > 0:
|
||||
animation_wait_start = time.monotonic()
|
||||
page.wait_for_timeout(tile_animation_wait * 1000)
|
||||
animation_wait_elapsed = time.monotonic() - animation_wait_start
|
||||
|
||||
# Per-tile timing breakdown so slow dashboards can be profiled from
|
||||
# logs alone. DEBUG rather than INFO: this fires once per tile, and
|
||||
# large dashboards can have dozens of tiles per report run.
|
||||
logger.debug(
|
||||
"Tile %s/%s timing: %.2fs waiting for chart readiness, "
|
||||
"%.2fs waiting for animations.%s",
|
||||
i + 1,
|
||||
num_tiles,
|
||||
readiness_wait_elapsed,
|
||||
animation_wait_elapsed,
|
||||
context_suffix,
|
||||
)
|
||||
|
||||
# Calculate what portion of the element we want to capture for this tile
|
||||
tile_start_in_element = i * tile_height
|
||||
@@ -431,6 +549,12 @@ def take_tiled_screenshot(
|
||||
|
||||
return combined_screenshot
|
||||
|
||||
except TiledScreenshotBudgetExceededError:
|
||||
# Budget exhaustion must fail cleanly, not be swallowed into the
|
||||
# generic `return None` degradation below -- the raise carries the
|
||||
# budget diagnostics to the caller, which fails the capture loudly
|
||||
# (#42273) instead of receiving an anonymous empty result.
|
||||
raise
|
||||
except Exception as e:
|
||||
if readiness_timeout:
|
||||
# Let the per-tile readiness timeout propagate so the caller
|
||||
|
||||
@@ -85,6 +85,26 @@ class ScreenshotCachePayloadType(TypedDict):
|
||||
status: str
|
||||
|
||||
|
||||
# Magic bytes for a cheap image sanity check. This is intentionally not a full
|
||||
# decode: it's meant to catch 0-byte/corrupt/blank payloads before they're
|
||||
# cached or served, not to validate the image is renderable.
|
||||
PNG_MAGIC_BYTES = b"\x89PNG\r\n\x1a\n"
|
||||
JPEG_MAGIC_BYTES = b"\xff\xd8\xff"
|
||||
|
||||
|
||||
def validate_screenshot_image(image: bytes | None) -> str | None:
|
||||
"""Cheaply validate screenshot bytes before they're cached or served.
|
||||
|
||||
:return: None if the bytes look like a usable image, otherwise a short
|
||||
reason ("empty" or "undecodable") suitable for logging.
|
||||
"""
|
||||
if not image:
|
||||
return "empty"
|
||||
if not image.startswith((PNG_MAGIC_BYTES, JPEG_MAGIC_BYTES)):
|
||||
return "undecodable"
|
||||
return None
|
||||
|
||||
|
||||
class ScreenshotCachePayload:
|
||||
def __init__(
|
||||
self,
|
||||
@@ -147,6 +167,13 @@ class ScreenshotCachePayload:
|
||||
def get_status(self) -> str:
|
||||
return self.status.value
|
||||
|
||||
def get_invalid_image_reason(self) -> str | None:
|
||||
"""Reason this payload's image should not be served/cached, or None if
|
||||
it passes validation (or it isn't claiming a successful screenshot)."""
|
||||
if self.status != StatusValues.UPDATED:
|
||||
return None
|
||||
return validate_screenshot_image(self._image)
|
||||
|
||||
def is_error_cache_ttl_expired(self) -> bool:
|
||||
error_cache_ttl = app.config["THUMBNAIL_ERROR_CACHE_TTL"]
|
||||
return (
|
||||
@@ -263,6 +290,14 @@ class BaseScreenshot:
|
||||
elif isinstance(payload, dict):
|
||||
payload = cast(ScreenshotCachePayloadType, payload)
|
||||
payload = ScreenshotCachePayload.from_dict(payload)
|
||||
if invalid_reason := payload.get_invalid_image_reason():
|
||||
logger.warning(
|
||||
"Rejecting cached screenshot for %s: %s image payload; "
|
||||
"treating as a cache miss",
|
||||
cache_key,
|
||||
invalid_reason,
|
||||
)
|
||||
return None
|
||||
return payload
|
||||
logger.info("Failed at getting from cache: %s", cache_key)
|
||||
return None
|
||||
@@ -331,15 +366,28 @@ class BaseScreenshot:
|
||||
image = None
|
||||
|
||||
# Cache the result (success or error) to avoid immediate retries
|
||||
if image:
|
||||
invalid_reason = validate_screenshot_image(image)
|
||||
# `image and` is redundant at runtime (validate_screenshot_image
|
||||
# only returns None for truthy, well-formed bytes) but mypy can't
|
||||
# infer that image is non-None from invalid_reason being None
|
||||
# across the function-call boundary, so it's kept for narrowing.
|
||||
if image and invalid_reason is None:
|
||||
with event_logger.log_context(
|
||||
f"screenshot.cache.{self.thumbnail_type}"
|
||||
):
|
||||
cache_payload.update(image)
|
||||
elif cache_payload.status != StatusValues.ERROR:
|
||||
# Only call error() if not already set — avoids overwriting
|
||||
# the timestamp recorded when the actual failure occurred above.
|
||||
cache_payload.error()
|
||||
else:
|
||||
if invalid_reason:
|
||||
logger.warning(
|
||||
"Not caching screenshot result for %s: %s image payload",
|
||||
cache_key,
|
||||
invalid_reason,
|
||||
)
|
||||
if cache_payload.status != StatusValues.ERROR:
|
||||
# Only call error() if not already set — avoids overwriting
|
||||
# the timestamp recorded when the actual failure occurred
|
||||
# above.
|
||||
cache_payload.error()
|
||||
|
||||
logger.info("Caching thumbnail: %s", cache_key)
|
||||
self.cache.set(cache_key, cache_payload.to_dict())
|
||||
|
||||
@@ -47,6 +47,7 @@ from superset.utils.screenshot_utils import (
|
||||
CHART_HOLDERS_READY_JS,
|
||||
FIND_CHART_HOLDER_STATES_JS,
|
||||
resolve_screenshot_task_budget_seconds,
|
||||
ScreenshotTaskBudgetExceededError,
|
||||
take_tiled_screenshot,
|
||||
)
|
||||
|
||||
@@ -61,10 +62,6 @@ PLAYWRIGHT_INSTALL_MESSAGE = (
|
||||
)
|
||||
|
||||
|
||||
class ScreenshotTaskBudgetExceededError(RuntimeError):
|
||||
"""Raised when no safe task budget remains before screenshot capture."""
|
||||
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from typing import Any
|
||||
|
||||
@@ -482,16 +479,31 @@ class WebDriverPlaywright(WebDriverProxy):
|
||||
logger.exception("Timed out requesting url %s", url)
|
||||
raise
|
||||
|
||||
slice_container_elems: list[Locator] = []
|
||||
rendered_chart_count = 0
|
||||
try:
|
||||
# chart containers didn't render
|
||||
logger.debug("Wait for chart containers to draw at url: %s", url)
|
||||
slice_container_locator = page.locator(".chart-container")
|
||||
for slice_container_elem in slice_container_locator.all():
|
||||
# One-time snapshot: containers mounting after this point
|
||||
# are neither waited on nor counted, so the progress
|
||||
# numbers below describe the snapshot, not the final DOM.
|
||||
slice_container_elems = slice_container_locator.all()
|
||||
for slice_container_elem in slice_container_elems:
|
||||
slice_container_elem.wait_for()
|
||||
rendered_chart_count += 1
|
||||
except PlaywrightTimeout:
|
||||
logger.exception(
|
||||
"Timed out waiting for chart containers to draw at url %s",
|
||||
# Customer-side chart loading is often just slow, not a
|
||||
# Superset bug, so this is a WARNING (matching the other
|
||||
# locate-wait timeouts below) rather than an ERROR -- but
|
||||
# it still fails the screenshot; see the `raise` below.
|
||||
logger.warning(
|
||||
"Timed out waiting for chart containers to draw at url %s "
|
||||
"(%s of %s chart containers rendered before the timeout)",
|
||||
url,
|
||||
rendered_chart_count,
|
||||
len(slice_container_elems),
|
||||
exc_info=True,
|
||||
)
|
||||
raise
|
||||
selenium_animation_wait = app.config[
|
||||
@@ -529,19 +541,38 @@ class WebDriverPlaywright(WebDriverProxy):
|
||||
"SCREENSHOT_TILED_VIEWPORT_HEIGHT", viewport_height
|
||||
)
|
||||
|
||||
if dashboard_height == 0:
|
||||
logger.warning(
|
||||
# A height of 0 means the DOM query above found no matching
|
||||
# element (or it hadn't laid out yet), not that the
|
||||
# dashboard is actually empty. Treat it as "unknown" rather
|
||||
# than "fits in a single tile": chart_count alone already
|
||||
# tells us whether this looks like a large dashboard, and
|
||||
# that signal must not be silently vetoed just because we
|
||||
# couldn't measure height, or a large dashboard could skip
|
||||
# tiling and ship with unrendered below-the-fold charts.
|
||||
height_unknown = dashboard_height == 0
|
||||
likely_large_dashboard = (
|
||||
chart_count >= chart_threshold
|
||||
or dashboard_height > height_threshold
|
||||
)
|
||||
if height_unknown:
|
||||
log_fn = (
|
||||
logger.warning if likely_large_dashboard else logger.debug
|
||||
)
|
||||
log_fn(
|
||||
"Could not determine dashboard height for element %s "
|
||||
"at url %s; falling back to standard screenshot behavior",
|
||||
"at url %s (%s chart containers found); %s",
|
||||
element_name,
|
||||
url,
|
||||
chart_count,
|
||||
"attempting tiled screenshot anyway"
|
||||
if likely_large_dashboard
|
||||
else "falling back to standard screenshot behavior",
|
||||
)
|
||||
|
||||
# Use tiled screenshots for large dashboards
|
||||
use_tiled = (
|
||||
chart_count >= chart_threshold
|
||||
or dashboard_height > height_threshold
|
||||
) and dashboard_height > tile_height
|
||||
use_tiled = likely_large_dashboard and (
|
||||
height_unknown or dashboard_height > tile_height
|
||||
)
|
||||
|
||||
if use_tiled:
|
||||
logger.info(
|
||||
@@ -563,18 +594,23 @@ class WebDriverPlaywright(WebDriverProxy):
|
||||
log_context=log_context,
|
||||
)
|
||||
if not img:
|
||||
# _get_screenshot() has no wait/readiness logic at
|
||||
# all, so falling back to it here would risk
|
||||
# silently delivering a screenshot of spinners or
|
||||
# a blank dashboard. Fail the capture loudly
|
||||
# (report error, thumbnail cache ERROR) instead of
|
||||
# guessing at a "safer" fallback.
|
||||
logger.warning(
|
||||
(
|
||||
"Tiled screenshot failed, "
|
||||
"falling back to standard screenshot"
|
||||
)
|
||||
"Tiled screenshot failed for url %s and no "
|
||||
"safe fallback exists; failing the capture",
|
||||
url,
|
||||
)
|
||||
img = WebDriverPlaywright._get_screenshot(
|
||||
page, element, element_name
|
||||
raise PlaywrightTimeout(
|
||||
f"Tiled screenshot failed for url {url}"
|
||||
)
|
||||
logger.debug(
|
||||
"Tiled screenshot result: %d bytes for url: %s",
|
||||
len(img) if img else 0,
|
||||
len(img),
|
||||
url,
|
||||
)
|
||||
else:
|
||||
|
||||
@@ -202,6 +202,23 @@ class Datasource(BaseSupersetView):
|
||||
payload = SamplesPayloadSchema().load(request.json)
|
||||
except ValidationError as err:
|
||||
return json_error_response(err.messages, status=400)
|
||||
|
||||
# Refuse early for datasource types that don't model raw rows
|
||||
# (e.g. semantic views, which only expose pre-defined metrics and
|
||||
# dimensions). Without this gate the request would still go through
|
||||
# the standard query pipeline and fail with an opaque 500.
|
||||
# ``supports_samples`` defaults to True for any datasource class that
|
||||
# doesn't explicitly opt out, so SqlaTable/Query/SavedQuery continue
|
||||
# to work without needing the attribute declared on each class.
|
||||
ds_class = DatasourceDAO.sources.get(
|
||||
DatasourceType(params["datasource_type"]),
|
||||
)
|
||||
if ds_class is not None and not getattr(ds_class, "supports_samples", True):
|
||||
return json_error_response(
|
||||
_("Samples are not available for this datasource type."),
|
||||
status=400,
|
||||
)
|
||||
|
||||
dashboard_id = None
|
||||
if security_manager.is_guest_user():
|
||||
if not params["dashboard_id"]:
|
||||
|
||||
@@ -26,7 +26,10 @@ from unittest.mock import patch
|
||||
import pytest
|
||||
|
||||
from superset.commands.report.base import BaseReportScheduleCommand
|
||||
from superset.commands.report.exceptions import ReportScheduleFrequencyNotAllowed
|
||||
from superset.commands.report.exceptions import (
|
||||
ReportScheduleCrontabNotValidError,
|
||||
ReportScheduleFrequencyNotAllowed,
|
||||
)
|
||||
from superset.reports.models import ReportScheduleType
|
||||
|
||||
REPORT_TYPES = {
|
||||
@@ -174,6 +177,28 @@ def test_validate_report_frequency_report_only(schedule: str) -> None:
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("report_type", REPORT_TYPES)
|
||||
@app_custom_config(
|
||||
alert_minimum_interval=int(timedelta(minutes=5).total_seconds()),
|
||||
report_minimum_interval=int(timedelta(minutes=5).total_seconds()),
|
||||
)
|
||||
def test_validate_report_frequency_never_matching_crontab(report_type: str) -> None:
|
||||
"""
|
||||
Test the ``validate_report_frequency`` method with a crontab that is
|
||||
syntactically valid but never matches a real calendar date (Feb 30th).
|
||||
|
||||
Such schedules pass ``croniter.is_valid()`` (purely syntactic) and thus
|
||||
marshmallow schema validation, but raise ``CroniterBadDateError`` when
|
||||
iterated. This should surface as a ``ValidationError`` rather than
|
||||
propagating the raw croniter exception.
|
||||
"""
|
||||
with pytest.raises(ReportScheduleCrontabNotValidError):
|
||||
BaseReportScheduleCommand().validate_report_frequency(
|
||||
"0 0 30 2 *",
|
||||
report_type,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("report_type", REPORT_TYPES)
|
||||
@pytest.mark.parametrize("schedule", TEST_SCHEDULES)
|
||||
@app_custom_config(
|
||||
|
||||
70
tests/unit_tests/common/test_query_actions_drill_detail.py
Normal file
70
tests/unit_tests/common/test_query_actions_drill_detail.py
Normal file
@@ -0,0 +1,70 @@
|
||||
# Licensed to the Apache Software Foundation (ASF) under one
|
||||
# or more contributor license agreements. See the NOTICE file
|
||||
# distributed with this work for additional information
|
||||
# regarding copyright ownership. The ASF licenses this file
|
||||
# to you under the Apache License, Version 2.0 (the
|
||||
# "License"); you may not use this file except in compliance
|
||||
# with the License. You may obtain a copy of the License at
|
||||
#
|
||||
# http://www.apache.org/licenses/LICENSE-2.0
|
||||
#
|
||||
# Unless required by applicable law or agreed to in writing,
|
||||
# software distributed under the License is distributed on an
|
||||
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
# KIND, either express or implied. See the License for the
|
||||
# specific language governing permissions and limitations
|
||||
# under the License.
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from superset.common.query_actions import _get_drill_detail
|
||||
from superset.exceptions import QueryObjectValidationError
|
||||
|
||||
|
||||
def test_get_drill_detail_refuses_datasource_that_opts_out() -> None:
|
||||
"""
|
||||
A datasource with ``supports_drill_to_detail = False`` (e.g. semantic
|
||||
views) must be hard-blocked on the server. Without this gate the request
|
||||
would fall through to ``_get_full`` and fail with an opaque error, and
|
||||
the flag would only be enforced by the frontend menu — leaving the
|
||||
chart-data API endpoint accepting drill-detail requests it shouldn't.
|
||||
"""
|
||||
datasource = MagicMock()
|
||||
datasource.supports_drill_to_detail = False
|
||||
|
||||
query_obj = MagicMock()
|
||||
query_obj.datasource = datasource
|
||||
|
||||
query_context = MagicMock()
|
||||
|
||||
with pytest.raises(
|
||||
QueryObjectValidationError,
|
||||
match="Drill to detail is not available",
|
||||
):
|
||||
_get_drill_detail(query_context, query_obj)
|
||||
|
||||
|
||||
def test_get_drill_detail_allows_datasource_without_flag() -> None:
|
||||
"""
|
||||
Datasources that don't declare the flag (e.g. legacy ``SqlaTable``
|
||||
subclasses via ``getattr`` default) must continue to work — the gate
|
||||
only fires when the flag is explicitly ``False``.
|
||||
"""
|
||||
datasource = MagicMock(spec=["columns"])
|
||||
column = MagicMock()
|
||||
column.column_name = "id"
|
||||
datasource.columns = [column]
|
||||
|
||||
query_obj = MagicMock()
|
||||
query_obj.datasource = datasource
|
||||
query_obj.columns = []
|
||||
|
||||
query_context = MagicMock()
|
||||
|
||||
expected_payload: dict[str, list[dict[str, str]]] = {"data": []}
|
||||
with patch(
|
||||
"superset.common.query_actions._get_full", return_value=expected_payload
|
||||
) as mock_get_full:
|
||||
assert _get_drill_detail(query_context, query_obj) is expected_payload
|
||||
mock_get_full.assert_called_once()
|
||||
@@ -2256,3 +2256,106 @@ def test_grouping_sets_fallback_applies_row_offset_once_globally() -> None:
|
||||
# ...and the requested offset is applied exactly once, to the combined
|
||||
# result: 2 + 1 = 3 total rows in, minus an offset of 1 = 2 rows out.
|
||||
assert len(result.df) == 2
|
||||
|
||||
|
||||
def test_relative_offset_preserves_inner_bounds(
|
||||
processor: QueryContextProcessor,
|
||||
) -> None:
|
||||
"""
|
||||
Regression test for #40501: Relative time comparison offset should
|
||||
preserve inner bounds as the original (unshifted) period, not the shifted one.
|
||||
|
||||
When comparing 2026-05-01 : 2026-05-28 with offset "365 days ago":
|
||||
- inner_from_dttm should be 2026-05-01 (original), NOT 2025-05-01 (shifted)
|
||||
- inner_to_dttm should be 2026-05-28 (original), NOT 2025-05-28 (shifted)
|
||||
"""
|
||||
from superset.common.query_object import QueryObject
|
||||
from superset.models.helpers import ExploreMixin
|
||||
|
||||
datasource: Any = processor._qc_datasource
|
||||
|
||||
for method in (
|
||||
"processing_time_offsets",
|
||||
"_align_offset_without_time_grain",
|
||||
"_coalesce_offset_index",
|
||||
):
|
||||
setattr(
|
||||
datasource,
|
||||
method,
|
||||
getattr(ExploreMixin, method).__get__(datasource),
|
||||
)
|
||||
|
||||
df = pd.DataFrame(
|
||||
{
|
||||
"__timestamp": pd.to_datetime(["2026-05-01", "2026-05-15", "2026-05-28"]),
|
||||
"sum__num": [100, 200, 300],
|
||||
}
|
||||
)
|
||||
|
||||
query_object = QueryObject(
|
||||
datasource=MagicMock(),
|
||||
granularity="ds",
|
||||
columns=[],
|
||||
metrics=["sum__num"],
|
||||
is_timeseries=True,
|
||||
time_offsets=["365 days ago"],
|
||||
filters=[
|
||||
{
|
||||
"col": "ds",
|
||||
"op": "TEMPORAL_RANGE",
|
||||
"val": "2026-05-01 : 2026-05-28",
|
||||
}
|
||||
],
|
||||
)
|
||||
|
||||
captured: list[dict[str, Any]] = []
|
||||
|
||||
def fake_query(dct: dict[str, Any]) -> MagicMock:
|
||||
captured.append(dct)
|
||||
result = MagicMock()
|
||||
result.df = pd.DataFrame(
|
||||
{
|
||||
"__timestamp": pd.date_range(
|
||||
start=dct["from_dttm"], periods=3, freq="14D"
|
||||
),
|
||||
"sum__num": [1.0, 2.0, 3.0],
|
||||
}
|
||||
)
|
||||
result.query = "SELECT 1"
|
||||
return result
|
||||
|
||||
datasource.query = fake_query
|
||||
datasource.normalize_df = MagicMock(
|
||||
side_effect=lambda offset_df, _query_object: offset_df
|
||||
)
|
||||
|
||||
with (
|
||||
patch(
|
||||
"superset.models.helpers.get_since_until_from_query_object",
|
||||
return_value=(pd.Timestamp("2026-05-01"), pd.Timestamp("2026-05-28")),
|
||||
),
|
||||
patch(
|
||||
"superset.common.utils.query_cache_manager.QueryCacheManager"
|
||||
) as mock_cache_manager,
|
||||
patch.object(
|
||||
datasource,
|
||||
"get_time_grain",
|
||||
return_value=None,
|
||||
),
|
||||
):
|
||||
mock_cache = MagicMock()
|
||||
mock_cache.is_loaded = False
|
||||
mock_cache_manager.get.return_value = mock_cache
|
||||
|
||||
datasource.processing_time_offsets(df, query_object, None, None, False)
|
||||
|
||||
# The offset query should use shifted dates for the main window
|
||||
assert len(captured) == 1
|
||||
assert captured[0]["from_dttm"] == pd.Timestamp("2025-05-01")
|
||||
assert captured[0]["to_dttm"] == pd.Timestamp("2025-05-28")
|
||||
|
||||
# The inner bounds (used for series-limit subquery) should be the
|
||||
# ORIGINAL unshifted dates, not the shifted ones — this is the fix
|
||||
# for #40501. Without the fix, inner_from/to_dttm == shifted dates.
|
||||
assert captured[0]["inner_from_dttm"] == pd.Timestamp("2026-05-01")
|
||||
assert captured[0]["inner_to_dttm"] == pd.Timestamp("2026-05-28")
|
||||
|
||||
@@ -39,13 +39,16 @@ def test_epoch_to_dttm() -> None:
|
||||
def test_get_table_comment(mocker: MockerFixture):
|
||||
"""
|
||||
Test the `get_table_comment` method.
|
||||
|
||||
ibm_db_sa >= 0.4.1 returns the comment as a plain string (fixed in
|
||||
https://github.com/ibmdb/python-ibmdbsa/pull/135), not a tuple as it
|
||||
used to. Indexing into that string with `comment[0]` truncates every
|
||||
DB2 table comment to its first character; this guards against that.
|
||||
"""
|
||||
from superset.db_engine_specs.db2 import Db2EngineSpec
|
||||
|
||||
mock_inspector = mocker.MagicMock()
|
||||
mock_inspector.get_table_comment.return_value = {
|
||||
"text": ("This is a table comment",)
|
||||
}
|
||||
mock_inspector.get_table_comment.return_value = {"text": "This is a table comment"}
|
||||
|
||||
assert (
|
||||
Db2EngineSpec.get_table_comment(mock_inspector, Table("my_table", "my_schema"))
|
||||
@@ -69,6 +72,22 @@ def test_get_table_comment_empty(mocker: MockerFixture):
|
||||
)
|
||||
|
||||
|
||||
def test_get_table_comment_unexpected_error(mocker: MockerFixture):
|
||||
"""
|
||||
Test that `get_table_comment` returns `None` instead of raising
|
||||
when the inspector call fails unexpectedly.
|
||||
"""
|
||||
from superset.db_engine_specs.db2 import Db2EngineSpec
|
||||
|
||||
mock_inspector = mocker.MagicMock()
|
||||
mock_inspector.get_table_comment.side_effect = Exception("boom")
|
||||
|
||||
assert (
|
||||
Db2EngineSpec.get_table_comment(mock_inspector, Table("my_table", "my_schema"))
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_get_prequeries(mocker: MockerFixture) -> None:
|
||||
"""
|
||||
Test the ``get_prequeries`` method.
|
||||
|
||||
@@ -450,3 +450,33 @@ def test_interval_type_mutator() -> None:
|
||||
assert mutator(True) is None
|
||||
assert mutator([1, 2, 3]) is None
|
||||
assert mutator({"days": 1}) is None
|
||||
|
||||
|
||||
def test_get_schema_names_excludes_only_actual_system_schemas(
|
||||
mocker: MockerFixture,
|
||||
) -> None:
|
||||
"""
|
||||
DB Eng Specs (postgres): Test ``get_schema_names``
|
||||
|
||||
User-defined schemas that merely start with ``pg`` (but are not
|
||||
actual Postgres system schemas, which always start with the literal
|
||||
``pg_``) must not be filtered out. See issue #30678.
|
||||
"""
|
||||
inspector = mocker.MagicMock()
|
||||
inspector.engine.connect().__enter__().execute.return_value = [
|
||||
("public",),
|
||||
("pgsql",),
|
||||
("pgstats",),
|
||||
("pg_catalog",),
|
||||
("pg_toast",),
|
||||
("information_schema",),
|
||||
]
|
||||
|
||||
schemas = spec.get_schema_names(inspector)
|
||||
|
||||
assert schemas == {
|
||||
"public",
|
||||
"pgsql",
|
||||
"pgstats",
|
||||
"information_schema",
|
||||
}
|
||||
|
||||
@@ -653,6 +653,15 @@ def test_semantic_view_data(
|
||||
assert data["table_name"] == "Orders View"
|
||||
assert data["datasource_name"] == "Orders View"
|
||||
assert data["offset"] == 0
|
||||
# Semantic views don't model raw rows, so neither samples nor
|
||||
# drill-to-detail are available.
|
||||
assert data["supports_samples"] is False
|
||||
assert data["supports_drill_to_detail"] is False
|
||||
|
||||
|
||||
def test_semantic_view_supports_samples_is_false() -> None:
|
||||
"""The class-level flag opts SemanticView out of the Samples affordance."""
|
||||
assert SemanticView.supports_samples is False
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@@ -767,6 +776,11 @@ def test_semantic_view_data_populates_time_grain_sqla(
|
||||
assert grain_durations == sorted(["PT1H", "P1D", "P1M"])
|
||||
|
||||
|
||||
def test_semantic_view_supports_drill_to_detail_is_false() -> None:
|
||||
"""The class-level flag opts SemanticView out of Drill to detail."""
|
||||
assert SemanticView.supports_drill_to_detail is False
|
||||
|
||||
|
||||
def test_semantic_view_get_query_result(
|
||||
mock_implementation: MagicMock,
|
||||
) -> None:
|
||||
|
||||
@@ -42,11 +42,11 @@ FIXED_USER_ID = 1234
|
||||
FIXED_USERNAME = "admin"
|
||||
|
||||
|
||||
def _make_user_subject(user_id: int) -> MagicMock:
|
||||
def _make_user_subject(user_id: int, active: bool = True) -> MagicMock:
|
||||
"""Create a mock user-type Subject with an underlying User."""
|
||||
from superset.subjects.types import SubjectType
|
||||
|
||||
user = User(id=user_id, username=str(user_id))
|
||||
user = User(id=user_id, username=str(user_id), active=active)
|
||||
subject = MagicMock()
|
||||
subject.id = user_id # deterministic subject ID
|
||||
subject.type = SubjectType.USER
|
||||
@@ -83,12 +83,13 @@ def _make_group_subject(group_id: int) -> MagicMock:
|
||||
|
||||
def _get_users(
|
||||
params: Optional[Union[int, list[int]]],
|
||||
active: bool = True,
|
||||
) -> Optional[Union[User, list[User]]]:
|
||||
if params is None:
|
||||
return None
|
||||
if isinstance(params, int):
|
||||
return User(id=params, username=str(params))
|
||||
return [User(id=user, username=str(user)) for user in params]
|
||||
return User(id=params, username=str(params), active=active)
|
||||
return [User(id=user, username=str(user), active=active) for user in params]
|
||||
|
||||
|
||||
@dataclass
|
||||
@@ -98,10 +99,14 @@ class EditorSpec:
|
||||
user_ids: list[int]
|
||||
role_ids: list[int] | None = None
|
||||
group_ids: list[int] | None = None
|
||||
# Direct user-type editors that should be built as inactive users
|
||||
inactive_user_ids: list[int] | None = None
|
||||
|
||||
def build(self) -> list[MagicMock]:
|
||||
editors: list[MagicMock] = []
|
||||
editors.extend(_make_user_subject(uid) for uid in self.user_ids)
|
||||
for uid in self.inactive_user_ids or []:
|
||||
editors.append(_make_user_subject(uid, active=False))
|
||||
for rid in self.role_ids or []:
|
||||
editors.append(_make_role_subject(rid))
|
||||
for gid in self.group_ids or []:
|
||||
@@ -114,6 +119,8 @@ class ModelConfig:
|
||||
editors: EditorSpec
|
||||
creator: Optional[int] = None
|
||||
modifier: Optional[int] = None
|
||||
creator_active: bool = True
|
||||
modifier_active: bool = True
|
||||
# Maps user_id → role_ids the user belongs to (for indirect editor resolution)
|
||||
user_roles: dict[int, list[int]] = field(default_factory=dict)
|
||||
# Maps user_id → group_ids the user belongs to (for indirect editor resolution)
|
||||
@@ -530,6 +537,105 @@ class ModelType(int, Enum):
|
||||
None,
|
||||
ExecutorNotFoundError(),
|
||||
),
|
||||
# CREATOR: an inactive creator is skipped (no other executor configured)
|
||||
(
|
||||
ModelType.REPORT_SCHEDULE,
|
||||
[ExecutorType.CREATOR],
|
||||
ModelConfig(
|
||||
editors=EditorSpec(user_ids=[]),
|
||||
creator=3,
|
||||
creator_active=False,
|
||||
),
|
||||
None,
|
||||
ExecutorNotFoundError(),
|
||||
),
|
||||
# CREATOR: an inactive creator is skipped, falling through to an active
|
||||
# MODIFIER later in the executor chain.
|
||||
(
|
||||
ModelType.REPORT_SCHEDULE,
|
||||
[ExecutorType.CREATOR, ExecutorType.MODIFIER],
|
||||
ModelConfig(
|
||||
editors=EditorSpec(user_ids=[]),
|
||||
creator=3,
|
||||
creator_active=False,
|
||||
modifier=4,
|
||||
),
|
||||
None,
|
||||
(ExecutorType.MODIFIER, 4),
|
||||
),
|
||||
# CREATOR_EDITOR: creator is an editor but inactive → not resolved
|
||||
(
|
||||
ModelType.REPORT_SCHEDULE,
|
||||
[ExecutorType.CREATOR_EDITOR],
|
||||
ModelConfig(
|
||||
editors=EditorSpec(user_ids=[4]),
|
||||
creator=4,
|
||||
creator_active=False,
|
||||
),
|
||||
None,
|
||||
ExecutorNotFoundError(),
|
||||
),
|
||||
# MODIFIER_EDITOR: modifier is an editor but inactive → not resolved
|
||||
(
|
||||
ModelType.REPORT_SCHEDULE,
|
||||
[ExecutorType.MODIFIER_EDITOR],
|
||||
ModelConfig(
|
||||
editors=EditorSpec(user_ids=[4]),
|
||||
modifier=4,
|
||||
modifier_active=False,
|
||||
),
|
||||
None,
|
||||
ExecutorNotFoundError(),
|
||||
),
|
||||
# EDITOR: creator (the only editor) is inactive, so scheduling falls
|
||||
# through to the still-active direct-user editor instead of failing.
|
||||
# This is the scenario from #33584: the original report creator has
|
||||
# gone inactive but a currently-active owner/editor should still be
|
||||
# usable as the executor.
|
||||
(
|
||||
ModelType.REPORT_SCHEDULE,
|
||||
[ExecutorType.EDITOR],
|
||||
ModelConfig(
|
||||
editors=EditorSpec(user_ids=[2], inactive_user_ids=[3]),
|
||||
creator=3,
|
||||
creator_active=False,
|
||||
),
|
||||
None,
|
||||
(ExecutorType.EDITOR, 2),
|
||||
),
|
||||
# EDITOR: both modifier and creator are inactive editors, and the only
|
||||
# direct-user editor is also inactive → falls through to the indirect
|
||||
# (role/group) editor resolution.
|
||||
(
|
||||
ModelType.REPORT_SCHEDULE,
|
||||
[ExecutorType.EDITOR],
|
||||
ModelConfig(
|
||||
editors=EditorSpec(
|
||||
user_ids=[], inactive_user_ids=[3, 4], role_ids=[10]
|
||||
),
|
||||
creator=3,
|
||||
creator_active=False,
|
||||
modifier=4,
|
||||
modifier_active=False,
|
||||
user_roles={6: [10]},
|
||||
),
|
||||
None,
|
||||
(ExecutorType.EDITOR, 6),
|
||||
),
|
||||
# EDITOR: modifier is an inactive editor, creator is an active editor →
|
||||
# resolves to the creator instead of failing outright.
|
||||
(
|
||||
ModelType.REPORT_SCHEDULE,
|
||||
[ExecutorType.EDITOR],
|
||||
ModelConfig(
|
||||
editors=EditorSpec(user_ids=[3], inactive_user_ids=[4]),
|
||||
creator=3,
|
||||
modifier=4,
|
||||
modifier_active=False,
|
||||
),
|
||||
None,
|
||||
(ExecutorType.EDITOR, 3),
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_get_executor(
|
||||
@@ -561,8 +667,10 @@ def test_get_executor(
|
||||
|
||||
obj = model(
|
||||
id=1,
|
||||
created_by=_get_users(model_config.creator),
|
||||
changed_by=_get_users(model_config.modifier),
|
||||
created_by=_get_users(model_config.creator, active=model_config.creator_active),
|
||||
changed_by=_get_users(
|
||||
model_config.modifier, active=model_config.modifier_active
|
||||
),
|
||||
**model_kwargs,
|
||||
)
|
||||
obj.editors = model_config.editors.build()
|
||||
|
||||
@@ -33,6 +33,10 @@ from superset.utils.screenshots import (
|
||||
|
||||
BASE_SCREENSHOT_PATH = "superset.utils.screenshots.BaseScreenshot"
|
||||
|
||||
# A minimal valid PNG header, used wherever a test needs bytes that pass
|
||||
# ScreenshotCachePayload's image validation.
|
||||
FAKE_PNG_BYTES = b"\x89PNG\r\n\x1a\n" + b"fake-png-body"
|
||||
|
||||
|
||||
class MockCache:
|
||||
"""A class to manage screenshot cache."""
|
||||
@@ -92,7 +96,7 @@ def test_get_cache_key(app_context, screenshot_obj):
|
||||
def test_get_from_cache_key(mocker: MockerFixture, screenshot_obj):
|
||||
"""get_from_cache_key should always return a ScreenshotCachePayload Object"""
|
||||
# backwards compatibility test for retrieving plain bytes
|
||||
fake_bytes = b"fake_screenshot_data"
|
||||
fake_bytes = FAKE_PNG_BYTES
|
||||
BaseScreenshot.cache = MockCache()
|
||||
BaseScreenshot.cache.set("key", fake_bytes)
|
||||
cache_payload = screenshot_obj.get_from_cache_key("key")
|
||||
@@ -108,10 +112,10 @@ class TestComputeAndCache:
|
||||
BASE_SCREENSHOT_PATH + ".get_from_cache_key", return_value=None
|
||||
)
|
||||
get_screenshot = mocker.patch(
|
||||
BASE_SCREENSHOT_PATH + ".get_screenshot", return_value=b"new_image_data"
|
||||
BASE_SCREENSHOT_PATH + ".get_screenshot", return_value=FAKE_PNG_BYTES
|
||||
)
|
||||
resize_image = mocker.patch(
|
||||
BASE_SCREENSHOT_PATH + ".resize_image", return_value=b"resized_image_data"
|
||||
BASE_SCREENSHOT_PATH + ".resize_image", return_value=FAKE_PNG_BYTES
|
||||
)
|
||||
BaseScreenshot.cache = MockCache()
|
||||
return {
|
||||
|
||||
@@ -37,6 +37,10 @@ from superset.utils.screenshots import (
|
||||
BASE_SCREENSHOT_PATH = "superset.utils.screenshots.BaseScreenshot"
|
||||
DISTRIBUTED_LOCK_PATH = "superset.utils.screenshots.DistributedLock"
|
||||
|
||||
# A minimal valid PNG header, used wherever a test needs bytes that pass
|
||||
# ScreenshotCachePayload's image validation.
|
||||
FAKE_PNG_BYTES = b"\x89PNG\r\n\x1a\n" + b"fake-png-body"
|
||||
|
||||
|
||||
class MockCache:
|
||||
"""A class to manage screenshot cache for testing."""
|
||||
@@ -83,11 +87,11 @@ class TestCacheOnlyOnSuccess:
|
||||
mocker.patch(DISTRIBUTED_LOCK_PATH)
|
||||
mocker.patch(BASE_SCREENSHOT_PATH + ".get_from_cache_key", return_value=None)
|
||||
get_screenshot = mocker.patch(
|
||||
BASE_SCREENSHOT_PATH + ".get_screenshot", return_value=b"image_data"
|
||||
BASE_SCREENSHOT_PATH + ".get_screenshot", return_value=FAKE_PNG_BYTES
|
||||
)
|
||||
# Mock resize_image to avoid PIL errors with fake image data
|
||||
mocker.patch(
|
||||
BASE_SCREENSHOT_PATH + ".resize_image", return_value=b"resized_image_data"
|
||||
BASE_SCREENSHOT_PATH + ".resize_image", return_value=FAKE_PNG_BYTES
|
||||
)
|
||||
BaseScreenshot.cache = MockCache()
|
||||
return get_screenshot
|
||||
@@ -161,13 +165,15 @@ class TestCacheOnlyOnSuccess:
|
||||
screenshot_obj: BaseScreenshot,
|
||||
mock_user: MagicMock,
|
||||
) -> None:
|
||||
"""Empty bytes from get_screenshot must set ERROR, not leave COMPUTING."""
|
||||
"""Empty bytes from get_screenshot must set ERROR, not leave COMPUTING,
|
||||
and must log a WARNING that includes the cache key."""
|
||||
mocker.patch(DISTRIBUTED_LOCK_PATH)
|
||||
mocker.patch(BASE_SCREENSHOT_PATH + ".get_from_cache_key", return_value=None)
|
||||
mocker.patch(
|
||||
BASE_SCREENSHOT_PATH + ".get_screenshot",
|
||||
return_value=b"",
|
||||
)
|
||||
mock_logger = mocker.patch("superset.utils.screenshots.logger")
|
||||
BaseScreenshot.cache = MockCache()
|
||||
|
||||
screenshot_obj.compute_and_cache(user=mock_user, force=True)
|
||||
@@ -177,6 +183,43 @@ class TestCacheOnlyOnSuccess:
|
||||
assert cached_value is not None
|
||||
assert cached_value["status"] == "Error"
|
||||
assert cached_value.get("image") is None
|
||||
assert any(
|
||||
cache_key in call.args and "empty" in call.args
|
||||
for call in mock_logger.warning.call_args_list
|
||||
)
|
||||
|
||||
def test_cache_error_status_when_screenshot_returns_garbage_bytes(
|
||||
self,
|
||||
mocker: MockerFixture,
|
||||
screenshot_obj: BaseScreenshot,
|
||||
mock_user: MagicMock,
|
||||
) -> None:
|
||||
"""Non-empty bytes without a valid image header must set ERROR, not be
|
||||
cached as a success, and must log a WARNING that includes the cache key."""
|
||||
mocker.patch(DISTRIBUTED_LOCK_PATH)
|
||||
mocker.patch(BASE_SCREENSHOT_PATH + ".get_from_cache_key", return_value=None)
|
||||
mocker.patch(
|
||||
BASE_SCREENSHOT_PATH + ".get_screenshot",
|
||||
return_value=b"this-is-not-a-real-image",
|
||||
)
|
||||
mocker.patch(
|
||||
BASE_SCREENSHOT_PATH + ".resize_image",
|
||||
return_value=b"this-is-not-a-real-image",
|
||||
)
|
||||
mock_logger = mocker.patch("superset.utils.screenshots.logger")
|
||||
BaseScreenshot.cache = MockCache()
|
||||
|
||||
screenshot_obj.compute_and_cache(user=mock_user, force=True)
|
||||
|
||||
cache_key = screenshot_obj.get_cache_key()
|
||||
cached_value = BaseScreenshot.cache.get(cache_key)
|
||||
assert cached_value is not None
|
||||
assert cached_value["status"] == "Error"
|
||||
assert cached_value.get("image") is None
|
||||
assert any(
|
||||
cache_key in call.args and "undecodable" in call.args
|
||||
for call in mock_logger.warning.call_args_list
|
||||
)
|
||||
|
||||
def test_computing_status_written_to_cache_early(
|
||||
self,
|
||||
@@ -197,14 +240,14 @@ class TestCacheOnlyOnSuccess:
|
||||
"Cache should be set to COMPUTING before screenshot starts"
|
||||
)
|
||||
assert cached_value["status"] == "Computing"
|
||||
return b"image_data"
|
||||
return FAKE_PNG_BYTES
|
||||
|
||||
mocker.patch(
|
||||
BASE_SCREENSHOT_PATH + ".get_screenshot",
|
||||
side_effect=check_cache_during_screenshot,
|
||||
)
|
||||
mocker.patch(
|
||||
BASE_SCREENSHOT_PATH + ".resize_image", return_value=b"resized_image_data"
|
||||
BASE_SCREENSHOT_PATH + ".resize_image", return_value=FAKE_PNG_BYTES
|
||||
)
|
||||
|
||||
screenshot_obj.compute_and_cache(user=mock_user, force=True)
|
||||
@@ -429,11 +472,11 @@ class TestIntegrationCacheBugFix:
|
||||
BaseScreenshot.cache.set(cache_key, stale_payload.to_dict())
|
||||
|
||||
mocker.patch(
|
||||
BASE_SCREENSHOT_PATH + ".get_screenshot", return_value=b"recovered_image"
|
||||
BASE_SCREENSHOT_PATH + ".get_screenshot", return_value=FAKE_PNG_BYTES
|
||||
)
|
||||
# Mock resize to avoid PIL errors
|
||||
mocker.patch(
|
||||
BASE_SCREENSHOT_PATH + ".resize_image", return_value=b"resized_image"
|
||||
BASE_SCREENSHOT_PATH + ".resize_image", return_value=FAKE_PNG_BYTES
|
||||
)
|
||||
|
||||
# Should trigger task because COMPUTING is stale
|
||||
@@ -482,3 +525,72 @@ class TestIntegrationCacheBugFix:
|
||||
|
||||
assert payload._image == old_image
|
||||
assert payload.status == StatusValues.COMPUTING
|
||||
|
||||
|
||||
class TestReadSideImageValidation:
|
||||
"""A cached payload that claims a successful screenshot (status UPDATED)
|
||||
but carries invalid image bytes must be served as a cache miss, not
|
||||
returned to the caller — this is what the dashboard/chart screenshot
|
||||
endpoints call to fetch bytes to serve."""
|
||||
|
||||
def test_zero_byte_image_is_treated_as_cache_miss(
|
||||
self, mocker: MockerFixture, screenshot_obj: BaseScreenshot
|
||||
) -> None:
|
||||
mock_logger = mocker.patch("superset.utils.screenshots.logger")
|
||||
BaseScreenshot.cache = MockCache()
|
||||
cache_key = screenshot_obj.get_cache_key()
|
||||
stale_payload = ScreenshotCachePayload(image=b"", status=StatusValues.UPDATED)
|
||||
BaseScreenshot.cache.set(cache_key, stale_payload.to_dict())
|
||||
|
||||
result = screenshot_obj.get_from_cache_key(cache_key)
|
||||
|
||||
assert result is None
|
||||
assert any(
|
||||
cache_key in call.args and "empty" in call.args
|
||||
for call in mock_logger.warning.call_args_list
|
||||
)
|
||||
|
||||
def test_garbage_bytes_image_is_treated_as_cache_miss(
|
||||
self, mocker: MockerFixture, screenshot_obj: BaseScreenshot
|
||||
) -> None:
|
||||
mock_logger = mocker.patch("superset.utils.screenshots.logger")
|
||||
BaseScreenshot.cache = MockCache()
|
||||
cache_key = screenshot_obj.get_cache_key()
|
||||
garbage_payload = ScreenshotCachePayload(image=b"not-an-image-at-all")
|
||||
BaseScreenshot.cache.set(cache_key, garbage_payload.to_dict())
|
||||
|
||||
result = screenshot_obj.get_from_cache_key(cache_key)
|
||||
|
||||
assert result is None
|
||||
assert any(
|
||||
cache_key in call.args and "undecodable" in call.args
|
||||
for call in mock_logger.warning.call_args_list
|
||||
)
|
||||
|
||||
def test_valid_image_is_served_normally(
|
||||
self, screenshot_obj: BaseScreenshot
|
||||
) -> None:
|
||||
BaseScreenshot.cache = MockCache()
|
||||
cache_key = screenshot_obj.get_cache_key()
|
||||
valid_payload = ScreenshotCachePayload(image=FAKE_PNG_BYTES)
|
||||
BaseScreenshot.cache.set(cache_key, valid_payload.to_dict())
|
||||
|
||||
result = screenshot_obj.get_from_cache_key(cache_key)
|
||||
|
||||
assert result is not None
|
||||
assert result.get_image().read() == FAKE_PNG_BYTES
|
||||
|
||||
def test_pending_status_with_no_image_is_not_rejected(
|
||||
self, screenshot_obj: BaseScreenshot
|
||||
) -> None:
|
||||
"""Non-UPDATED statuses (e.g. PENDING/COMPUTING) aren't claiming a
|
||||
successful screenshot, so they should be returned as-is."""
|
||||
BaseScreenshot.cache = MockCache()
|
||||
cache_key = screenshot_obj.get_cache_key()
|
||||
pending_payload = ScreenshotCachePayload(status=StatusValues.PENDING)
|
||||
BaseScreenshot.cache.set(cache_key, pending_payload.to_dict())
|
||||
|
||||
result = screenshot_obj.get_from_cache_key(cache_key)
|
||||
|
||||
assert result is not None
|
||||
assert result.status == StatusValues.PENDING
|
||||
|
||||
@@ -25,8 +25,11 @@ from superset.utils.screenshot_utils import (
|
||||
combine_screenshot_tiles,
|
||||
resolve_screenshot_task_budget_seconds,
|
||||
SCREENSHOT_TASK_BUDGET_MAX_MARGIN_SECONDS,
|
||||
ScreenshotTaskBudgetExceededError,
|
||||
SCROLL_SETTLE_TIMEOUT_MS,
|
||||
take_tiled_screenshot,
|
||||
TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS,
|
||||
TiledScreenshotBudgetExceededError,
|
||||
)
|
||||
|
||||
|
||||
@@ -453,12 +456,17 @@ class TestTakeTiledScreenshot:
|
||||
assert warning_args[2] == 1 # count of unready chart containers
|
||||
assert warning_args[3] == 1 # tile index
|
||||
assert warning_args[4] == 3 # total tiles
|
||||
assert warning_args[5] == 30 # load_wait
|
||||
assert warning_args[6] == "" # no log_context passed
|
||||
assert warning_args[5] == 30 # tile_load_wait (uncapped: budget remains)
|
||||
assert warning_args[6] == 30 # requested load_wait
|
||||
assert isinstance(warning_args[7], float) # total elapsed vs budget
|
||||
assert warning_args[8] == 1440 # total budget (fixed fallback)
|
||||
assert warning_args[9] == 0 # tiles captured so far
|
||||
assert warning_args[10] == 3 # total tiles
|
||||
assert warning_args[11] == "" # no log_context passed
|
||||
# Diagnostic payload identifies chart id AND the state it's stuck in
|
||||
# (spinner mounted vs nothing mounted vs waiting-on-database) so a
|
||||
# slow query can be told apart from the virtualization race.
|
||||
assert warning_args[7] == [{"chartId": "42", "state": "waiting_on_database"}]
|
||||
assert warning_args[12] == [{"chartId": "42", "state": "waiting_on_database"}]
|
||||
|
||||
def test_timeout_warning_includes_log_context(self, mock_page):
|
||||
"""The log context (e.g. report execution id) is threaded through for
|
||||
@@ -484,7 +492,7 @@ class TestTakeTiledScreenshot:
|
||||
)
|
||||
|
||||
warning_args = mock_logger.warning.call_args[0]
|
||||
assert warning_args[6] == " [execution_id=abc-123]"
|
||||
assert warning_args[11] == " [execution_id=abc-123]"
|
||||
|
||||
def test_chart_holder_with_nothing_mounted_blocks_wait(self, mock_page):
|
||||
"""Regression test for the vacuous-pass race (PR #39895).
|
||||
@@ -646,3 +654,311 @@ class TestTakeTiledScreenshot:
|
||||
|
||||
sig = inspect.signature(take_tiled_screenshot)
|
||||
assert sig.parameters["animation_wait"].default == 0
|
||||
|
||||
|
||||
class TestTileWaitBudget:
|
||||
"""The tiled operation's cumulative per-tile waits are capped by one
|
||||
wall-clock budget derived from the running Celery task's own time limit
|
||||
(resolve_screenshot_task_budget_seconds), falling back to a fixed total
|
||||
ceiling outside Celery because per-tile waits accumulate."""
|
||||
|
||||
@pytest.fixture
|
||||
def mock_page(self):
|
||||
"""Create a mock Playwright page object for a 3-tile (5000px) dashboard."""
|
||||
page = MagicMock()
|
||||
element = MagicMock()
|
||||
page.locator.return_value = element
|
||||
page.evaluate.return_value = {
|
||||
"height": 5000,
|
||||
"top": 100,
|
||||
"left": 50,
|
||||
"width": 800,
|
||||
}
|
||||
page.screenshot.return_value = b"fake_screenshot_data"
|
||||
return page
|
||||
|
||||
class _FakeClock:
|
||||
"""Stateful monotonic() stand-in the test advances explicitly.
|
||||
|
||||
Robust to how many times the code under test samples the clock per
|
||||
tile (budget check, per-tile wait timing, animation budget) -- only
|
||||
explicit advances move time forward.
|
||||
"""
|
||||
|
||||
def __init__(self) -> None:
|
||||
self.now = 0.0
|
||||
|
||||
def __call__(self) -> float:
|
||||
return self.now
|
||||
|
||||
def test_budget_error_is_task_budget_error_subclass(self):
|
||||
"""Callers can catch the whole budget-error family with the base
|
||||
ScreenshotTaskBudgetExceededError type."""
|
||||
assert issubclass(
|
||||
TiledScreenshotBudgetExceededError, ScreenshotTaskBudgetExceededError
|
||||
)
|
||||
|
||||
def test_per_tile_wait_shrinks_as_budget_depletes(self, mock_page, monkeypatch):
|
||||
"""Each tile's readiness-wait timeout is capped at the remaining budget."""
|
||||
monkeypatch.setattr(
|
||||
"superset.utils.screenshot_utils.TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS", # noqa: E501
|
||||
1000,
|
||||
)
|
||||
clock = self._FakeClock()
|
||||
# Simulate slow tiles: the readiness wait itself consumes wall time,
|
||||
# so each subsequent tile sees less remaining budget.
|
||||
wait_durations = iter([950, 40, 5])
|
||||
|
||||
def slow_wait(*args, **kwargs):
|
||||
clock.now += next(wait_durations)
|
||||
|
||||
mock_page.wait_for_function.side_effect = slow_wait
|
||||
|
||||
with patch("superset.utils.screenshot_utils.current_task", None):
|
||||
with patch("superset.utils.screenshot_utils.time.monotonic", new=clock):
|
||||
with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
|
||||
result = take_tiled_screenshot(
|
||||
mock_page, "dashboard", tile_height=2000, load_wait=100
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
timeouts = [
|
||||
call[1]["timeout"] for call in mock_page.wait_for_function.call_args_list
|
||||
]
|
||||
# remaining budget at each tile's wait: 1000, 50, 10 seconds
|
||||
# -> capped timeouts shrink
|
||||
assert timeouts == [100 * 1000, 50 * 1000, 10 * 1000]
|
||||
assert timeouts == sorted(timeouts, reverse=True)
|
||||
|
||||
def test_readiness_wait_uses_budget_recomputed_after_scroll_settle(
|
||||
self, mock_page, monkeypatch
|
||||
):
|
||||
"""The readiness-wait timeout must be capped using the budget
|
||||
recomputed *after* the scroll-settle sleep, not the stale value from
|
||||
before it -- otherwise each tile could overrun the total budget by up
|
||||
to one settle interval."""
|
||||
monkeypatch.setattr(
|
||||
"superset.utils.screenshot_utils.TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS", # noqa: E501
|
||||
1000,
|
||||
)
|
||||
# A single-tile dashboard to keep the scenario simple.
|
||||
mock_page.evaluate.return_value = {
|
||||
"height": 1000,
|
||||
"top": 100,
|
||||
"left": 50,
|
||||
"width": 800,
|
||||
}
|
||||
clock = self._FakeClock()
|
||||
# The scroll-settle sleep itself consumes 950s of wall-clock time,
|
||||
# leaving only 50s of the 1000s budget by the time the readiness
|
||||
# wait is capped.
|
||||
mock_page.wait_for_timeout.side_effect = lambda *args, **kwargs: setattr(
|
||||
clock, "now", clock.now + 950
|
||||
)
|
||||
|
||||
with patch("superset.utils.screenshot_utils.current_task", None):
|
||||
with patch("superset.utils.screenshot_utils.time.monotonic", new=clock):
|
||||
with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
|
||||
take_tiled_screenshot(
|
||||
mock_page, "dashboard", tile_height=2000, load_wait=999
|
||||
)
|
||||
|
||||
timeout = mock_page.wait_for_function.call_args_list[0][1]["timeout"]
|
||||
# Must reflect the post-settle remaining budget (50s), not the
|
||||
# stale pre-settle value (1000s, which would have let load_wait's
|
||||
# full 999s through uncapped).
|
||||
assert timeout == 50 * 1000
|
||||
|
||||
def test_budget_exhausted_raises_and_stops_capturing(self, mock_page, monkeypatch):
|
||||
"""Exhausting the budget aborts cleanly instead of capturing unchecked."""
|
||||
monkeypatch.setattr(
|
||||
"superset.utils.screenshot_utils.TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS", # noqa: E501
|
||||
1000,
|
||||
)
|
||||
clock = self._FakeClock()
|
||||
# Tile 0's readiness wait consumes the whole budget; tile 1's budget
|
||||
# check then sees remaining <= 0 and raises before capturing.
|
||||
mock_page.wait_for_function.side_effect = lambda *args, **kwargs: setattr(
|
||||
clock, "now", 1000.0
|
||||
)
|
||||
|
||||
with patch("superset.utils.screenshot_utils.current_task", None):
|
||||
with patch("superset.utils.screenshot_utils.time.monotonic", new=clock):
|
||||
with patch(
|
||||
"superset.utils.screenshot_utils.combine_screenshot_tiles"
|
||||
) as mock_combine:
|
||||
with patch("superset.utils.screenshot_utils.logger") as mock_logger:
|
||||
with pytest.raises(TiledScreenshotBudgetExceededError):
|
||||
take_tiled_screenshot(
|
||||
mock_page, "dashboard", tile_height=2000, load_wait=100
|
||||
)
|
||||
|
||||
# Only the first tile was captured before the budget ran out.
|
||||
assert mock_page.screenshot.call_count == 1
|
||||
# Tiles were never combined -- the function raised before that point.
|
||||
mock_combine.assert_not_called()
|
||||
|
||||
# Budget exhaustion is a customer chart-loading issue, not a Superset
|
||||
# system fault, so it must log at WARNING (not ERROR) -- consistent
|
||||
# with the #38130/#38441 precedent for screenshot timeout logging.
|
||||
assert mock_logger.error.call_count == 0
|
||||
mock_logger.warning.assert_called_once()
|
||||
warning_args = mock_logger.warning.call_args[0]
|
||||
assert "budget exhausted" in warning_args[0]
|
||||
# tile index, tiles total, tiles captured, tiles total,
|
||||
# elapsed seconds, budget seconds, log-context suffix
|
||||
assert warning_args[1] == 2
|
||||
assert warning_args[2] == 3
|
||||
assert warning_args[3] == 1
|
||||
assert warning_args[4] == 3
|
||||
assert warning_args[5] == 1000
|
||||
assert warning_args[6] == 1000
|
||||
assert warning_args[7] == ""
|
||||
|
||||
def test_budget_exhausted_warning_includes_log_context(
|
||||
self, mock_page, monkeypatch
|
||||
):
|
||||
"""log_context (e.g. report execution id) is appended to the warning."""
|
||||
monkeypatch.setattr(
|
||||
"superset.utils.screenshot_utils.TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS", # noqa: E501
|
||||
1000,
|
||||
)
|
||||
clock = self._FakeClock()
|
||||
# Tile 0's readiness wait consumes the whole budget; tile 1's budget
|
||||
# check then sees remaining <= 0 and raises.
|
||||
mock_page.wait_for_function.side_effect = lambda *args, **kwargs: setattr(
|
||||
clock, "now", 1000.0
|
||||
)
|
||||
with patch("superset.utils.screenshot_utils.current_task", None):
|
||||
with patch("superset.utils.screenshot_utils.time.monotonic", new=clock):
|
||||
with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
|
||||
with patch("superset.utils.screenshot_utils.logger") as mock_logger:
|
||||
with pytest.raises(TiledScreenshotBudgetExceededError):
|
||||
take_tiled_screenshot(
|
||||
mock_page,
|
||||
"dashboard",
|
||||
tile_height=2000,
|
||||
load_wait=100,
|
||||
log_context="execution_id=abc-123",
|
||||
)
|
||||
|
||||
warning_args = mock_logger.warning.call_args[0]
|
||||
assert warning_args[-1] == " [execution_id=abc-123]"
|
||||
|
||||
def test_budget_exhausted_before_first_tile_raises_without_capture(
|
||||
self, mock_page, monkeypatch
|
||||
):
|
||||
"""No budget floor: a budget already exhausted by setup (element
|
||||
lookup/dimension probing) raises before the first tile is captured,
|
||||
matching the non-tiled path's raise-before-capture semantics."""
|
||||
monkeypatch.setattr(
|
||||
"superset.utils.screenshot_utils.TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS", # noqa: E501
|
||||
1000,
|
||||
)
|
||||
clock = self._FakeClock()
|
||||
# The dashboard-dimension evaluate() itself consumes the whole budget.
|
||||
original_return = {"height": 5000, "top": 100, "left": 50, "width": 800}
|
||||
|
||||
def slow_evaluate(*args, **kwargs):
|
||||
clock.now = 1000.0
|
||||
return original_return
|
||||
|
||||
mock_page.evaluate.side_effect = slow_evaluate
|
||||
|
||||
with patch("superset.utils.screenshot_utils.current_task", None):
|
||||
with patch("superset.utils.screenshot_utils.time.monotonic", new=clock):
|
||||
with patch(
|
||||
"superset.utils.screenshot_utils.combine_screenshot_tiles"
|
||||
) as mock_combine:
|
||||
with pytest.raises(TiledScreenshotBudgetExceededError):
|
||||
take_tiled_screenshot(
|
||||
mock_page, "dashboard", tile_height=2000, load_wait=100
|
||||
)
|
||||
|
||||
mock_page.screenshot.assert_not_called()
|
||||
mock_combine.assert_not_called()
|
||||
|
||||
def test_no_celery_context_uses_fixed_total_fallback(self, mock_page):
|
||||
"""Outside Celery the helper returns None; the tiled path must fall
|
||||
back to the fixed total ceiling rather than running uncapped, because
|
||||
per-tile waits accumulate across tiles."""
|
||||
clock = self._FakeClock()
|
||||
with patch("superset.utils.screenshot_utils.current_task", None):
|
||||
with patch("superset.utils.screenshot_utils.time.monotonic", new=clock):
|
||||
with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
|
||||
take_tiled_screenshot(
|
||||
mock_page,
|
||||
"dashboard",
|
||||
tile_height=2000,
|
||||
load_wait=10_000, # deliberately above the fallback
|
||||
)
|
||||
|
||||
first_timeout = mock_page.wait_for_function.call_args_list[0][1]["timeout"]
|
||||
assert first_timeout == TILED_SCREENSHOT_TOTAL_WAIT_BUDGET_SECONDS * 1000
|
||||
|
||||
def test_derived_task_budget_caps_tile_wait(self, mock_page):
|
||||
"""Inside Celery, the tiled path caps waits using the same
|
||||
task-derived budget as the non-tiled path (helper reuse, #42427)."""
|
||||
task = MagicMock()
|
||||
task.request.timelimit = (120, None) # (hard, soft): 120s hard limit
|
||||
|
||||
clock = self._FakeClock()
|
||||
with patch("superset.utils.screenshot_utils.current_task", task):
|
||||
with patch("superset.utils.screenshot_utils.time.monotonic", new=clock):
|
||||
with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
|
||||
take_tiled_screenshot(
|
||||
mock_page, "dashboard", tile_height=2000, load_wait=200
|
||||
)
|
||||
|
||||
# margin = min(300, 120 * 0.2) = 24; budget = 120 - 24 = 96
|
||||
first_timeout = mock_page.wait_for_function.call_args_list[0][1]["timeout"]
|
||||
assert first_timeout == 96 * 1000
|
||||
assert first_timeout < 200 * 1000
|
||||
|
||||
def test_fast_dashboard_matches_default_behavior(self, mock_page):
|
||||
"""Well under budget, waits are not capped and behavior is unchanged."""
|
||||
with patch("superset.utils.screenshot_utils.current_task", None):
|
||||
with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
|
||||
result = take_tiled_screenshot(
|
||||
mock_page,
|
||||
"dashboard",
|
||||
tile_height=2000,
|
||||
load_wait=30,
|
||||
animation_wait=5,
|
||||
)
|
||||
|
||||
assert result is not None
|
||||
assert mock_page.screenshot.call_count == 3
|
||||
|
||||
for call in mock_page.wait_for_function.call_args_list:
|
||||
assert call[1]["timeout"] == 30 * 1000
|
||||
|
||||
animation_calls = [
|
||||
call
|
||||
for call in mock_page.wait_for_timeout.call_args_list
|
||||
if call[0][0] == 5 * 1000
|
||||
]
|
||||
assert len(animation_calls) == 3
|
||||
|
||||
def test_per_tile_timing_debug_line_logged(self, mock_page):
|
||||
"""Each tile logs a DEBUG timing breakdown (readiness wait, animation
|
||||
wait) so slow dashboards can be profiled from logs alone."""
|
||||
with patch("superset.utils.screenshot_utils.current_task", None):
|
||||
with patch("superset.utils.screenshot_utils.logger") as mock_logger:
|
||||
with patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
|
||||
take_tiled_screenshot(
|
||||
mock_page,
|
||||
"dashboard",
|
||||
tile_height=2000,
|
||||
log_context="cache_key=xyz",
|
||||
)
|
||||
|
||||
timing_calls = [
|
||||
call for call in mock_logger.debug.call_args_list if "timing" in call[0][0]
|
||||
]
|
||||
assert len(timing_calls) == 3
|
||||
for i, call in enumerate(timing_calls):
|
||||
args = call[0]
|
||||
assert args[1] == i + 1 # tile index
|
||||
assert args[2] == 3 # total tiles
|
||||
assert args[-1] == " [cache_key=xyz]"
|
||||
|
||||
@@ -919,21 +919,197 @@ class TestWebDriverPlaywrightErrorHandling:
|
||||
)
|
||||
|
||||
assert result == b"fake_screenshot"
|
||||
mock_logger.warning.assert_any_call(
|
||||
"Could not determine dashboard height for element %s at url %s; "
|
||||
"falling back to standard screenshot behavior",
|
||||
# chart_count (1) is well below the tiling threshold (20), so this is
|
||||
# the benign/expected case and must not be logged as a WARNING.
|
||||
mock_logger.debug.assert_any_call(
|
||||
"Could not determine dashboard height for element %s "
|
||||
"at url %s (%s chart containers found); %s",
|
||||
"dashboard",
|
||||
"http://example.com",
|
||||
1,
|
||||
"falling back to standard screenshot behavior",
|
||||
)
|
||||
assert not any(
|
||||
call.args and "Could not determine dashboard height" in call.args[0]
|
||||
for call in mock_logger.warning.call_args_list
|
||||
)
|
||||
|
||||
@patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
|
||||
@patch("superset.utils.webdriver._browser_manager")
|
||||
@patch("superset.utils.webdriver.logger")
|
||||
@patch("superset.utils.webdriver.take_tiled_screenshot")
|
||||
def test_tiled_screenshot_failure_falls_back_to_standard_screenshot(
|
||||
def test_unknown_height_does_not_veto_tiling_for_large_dashboard(
|
||||
self, mock_take_tiled, mock_logger, mock_browser_manager
|
||||
):
|
||||
"""
|
||||
A large dashboard (by chart_count) whose height can't be measured
|
||||
must still attempt tiling instead of being silently downgraded to a
|
||||
standard screenshot, since below-the-fold charts may not have
|
||||
rendered without the scroll-driven tiling pass.
|
||||
"""
|
||||
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_element.wait_for.return_value = None
|
||||
mock_chart_container.wait_for.return_value = None
|
||||
mock_page.wait_for_timeout.return_value = None
|
||||
mock_take_tiled.return_value = b"tiled_screenshot"
|
||||
|
||||
def evaluate_side_effect(script):
|
||||
if script == 'document.querySelectorAll(".chart-container").length':
|
||||
return 25 # chart_count >= threshold
|
||||
if "const target = document.querySelector" in script:
|
||||
return 0 # height could not be determined
|
||||
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": 5,
|
||||
"SCREENSHOT_SELENIUM_ANIMATION_WAIT": 1,
|
||||
"SCREENSHOT_LOCATE_WAIT": 10,
|
||||
"SCREENSHOT_LOAD_WAIT": 10,
|
||||
"SCREENSHOT_WAIT_FOR_ERROR_MODAL_VISIBLE": 10,
|
||||
"SCREENSHOT_WAIT_FOR_ERROR_MODAL_INVISIBLE": 10,
|
||||
"SCREENSHOT_REPLACE_UNEXPECTED_ERRORS": False,
|
||||
"SCREENSHOT_TILED_ENABLED": True,
|
||||
"SCREENSHOT_TILED_CHART_THRESHOLD": 20,
|
||||
"SCREENSHOT_TILED_HEIGHT_THRESHOLD": 5000,
|
||||
"SCREENSHOT_TILED_VIEWPORT_HEIGHT": 600,
|
||||
}
|
||||
|
||||
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", mock_user
|
||||
)
|
||||
|
||||
assert result == b"tiled_screenshot"
|
||||
mock_take_tiled.assert_called_once()
|
||||
mock_logger.warning.assert_any_call(
|
||||
"Could not determine dashboard height for element %s "
|
||||
"at url %s (%s chart containers found); %s",
|
||||
"dashboard",
|
||||
"http://example.com",
|
||||
25,
|
||||
"attempting tiled screenshot anyway",
|
||||
)
|
||||
|
||||
@patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
|
||||
@patch("superset.utils.webdriver._browser_manager")
|
||||
@patch("superset.utils.webdriver.logger")
|
||||
def test_chart_container_timeout_logs_warning_with_progress_and_raises(
|
||||
self, mock_logger, mock_browser_manager
|
||||
):
|
||||
"""
|
||||
Timing out while waiting for `.chart-container` elements to draw must
|
||||
be logged as a WARNING (matching the other locate-wait timeouts in
|
||||
this method, and the customer-side-slowness convention established
|
||||
for these Playwright timeouts) with rendered/total progress, and must
|
||||
still fail the screenshot by re-raising.
|
||||
"""
|
||||
from superset.utils.webdriver import PlaywrightTimeout
|
||||
|
||||
mock_user = MagicMock()
|
||||
mock_user.username = "test_user"
|
||||
|
||||
mock_browser = MagicMock()
|
||||
mock_context = MagicMock()
|
||||
mock_page = MagicMock()
|
||||
mock_element = 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
|
||||
|
||||
timeout = PlaywrightTimeout()
|
||||
rendered_ok = MagicMock()
|
||||
rendered_ok.wait_for.return_value = None
|
||||
never_renders = MagicMock()
|
||||
never_renders.wait_for.side_effect = timeout
|
||||
|
||||
def locator_side_effect(selector):
|
||||
if selector == ".chart-container":
|
||||
locator = MagicMock()
|
||||
locator.all.return_value = [rendered_ok, never_renders]
|
||||
return locator
|
||||
return mock_element
|
||||
|
||||
mock_page.locator.side_effect = locator_side_effect
|
||||
mock_element.wait_for.return_value = None
|
||||
|
||||
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": 5,
|
||||
"SCREENSHOT_SELENIUM_ANIMATION_WAIT": 1,
|
||||
"SCREENSHOT_LOCATE_WAIT": 10,
|
||||
"SCREENSHOT_LOAD_WAIT": 10,
|
||||
"SCREENSHOT_WAIT_FOR_ERROR_MODAL_VISIBLE": 10,
|
||||
"SCREENSHOT_WAIT_FOR_ERROR_MODAL_INVISIBLE": 10,
|
||||
"SCREENSHOT_REPLACE_UNEXPECTED_ERRORS": False,
|
||||
"SCREENSHOT_TILED_ENABLED": False,
|
||||
}
|
||||
|
||||
with patch.object(WebDriverPlaywright, "auth") as mock_auth:
|
||||
mock_auth.return_value = mock_context
|
||||
|
||||
driver = WebDriverPlaywright("chrome")
|
||||
with pytest.raises(PlaywrightTimeout) as exc_info:
|
||||
driver.get_screenshot(
|
||||
"http://example.com", "test-element", mock_user
|
||||
)
|
||||
|
||||
assert exc_info.value is timeout
|
||||
mock_logger.warning.assert_any_call(
|
||||
"Timed out waiting for chart containers to draw at url %s "
|
||||
"(%s of %s chart containers rendered before the timeout)",
|
||||
"http://example.com",
|
||||
1,
|
||||
2,
|
||||
exc_info=True,
|
||||
)
|
||||
mock_logger.exception.assert_not_called()
|
||||
|
||||
@patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
|
||||
@patch("superset.utils.webdriver._browser_manager")
|
||||
@patch("superset.utils.webdriver.logger")
|
||||
@patch("superset.utils.webdriver.take_tiled_screenshot")
|
||||
def test_tiled_screenshot_failure_raises_without_fallback(
|
||||
self, mock_take_tiled, mock_logger, mock_browser_manager
|
||||
) -> None:
|
||||
"""When take_tiled_screenshot returns None, fall back to standard screenshot."""
|
||||
"""When take_tiled_screenshot returns None, fail loudly instead of
|
||||
falling back to an unguarded standard screenshot."""
|
||||
from superset.utils.webdriver import PlaywrightTimeout
|
||||
|
||||
mock_user = MagicMock()
|
||||
mock_user.username = "test_user"
|
||||
|
||||
@@ -947,7 +1123,8 @@ class TestWebDriverPlaywrightErrorHandling:
|
||||
mock_context.new_page.return_value = mock_page
|
||||
mock_page.locator.return_value = mock_element
|
||||
mock_element.wait_for.return_value = None
|
||||
# page.screenshot is used by _get_screenshot for the "standalone" element
|
||||
# page.screenshot is used by _get_screenshot for the "standalone" element;
|
||||
# it must never be reached by the failure path under test.
|
||||
mock_page.screenshot.return_value = b"fallback_screenshot"
|
||||
|
||||
def evaluate_side_effect(script):
|
||||
@@ -983,14 +1160,20 @@ class TestWebDriverPlaywrightErrorHandling:
|
||||
mock_auth.return_value = mock_context
|
||||
|
||||
driver = WebDriverPlaywright("chrome")
|
||||
result = driver.get_screenshot(
|
||||
"http://example.com", "standalone", mock_user
|
||||
)
|
||||
# match= keeps this assertion meaningful even when playwright
|
||||
# is not installed and PlaywrightTimeout aliases bare Exception.
|
||||
with pytest.raises(
|
||||
PlaywrightTimeout, match="Tiled screenshot failed for url"
|
||||
):
|
||||
driver.get_screenshot("http://example.com", "standalone", mock_user)
|
||||
|
||||
assert result == b"fallback_screenshot"
|
||||
mock_take_tiled.assert_called_once()
|
||||
mock_page.screenshot.assert_not_called()
|
||||
mock_element.screenshot.assert_not_called()
|
||||
mock_logger.warning.assert_any_call(
|
||||
("Tiled screenshot failed, falling back to standard screenshot"),
|
||||
"Tiled screenshot failed for url %s and no safe fallback "
|
||||
"exists; failing the capture",
|
||||
"http://example.com",
|
||||
)
|
||||
|
||||
|
||||
@@ -1514,10 +1697,13 @@ class TestWebDriverPlaywrightAnimationWaitOrder:
|
||||
@patch("superset.utils.webdriver._browser_manager")
|
||||
@patch("superset.utils.webdriver.take_tiled_screenshot")
|
||||
@patch("superset.utils.webdriver.app")
|
||||
def test_tiled_fallback_triggered_on_empty_bytes(
|
||||
def test_tiled_empty_bytes_raises_without_fallback(
|
||||
self, mock_app, mock_take_tiled, mock_browser_manager
|
||||
):
|
||||
"""Tiled fallback fires when take_tiled_screenshot returns b"" (not None)."""
|
||||
"""Tiled failure raises when take_tiled_screenshot returns b"" (not None),
|
||||
instead of silently falling through to an unguarded raw capture."""
|
||||
from superset.utils.webdriver import PlaywrightTimeout
|
||||
|
||||
mock_user = MagicMock()
|
||||
mock_user.username = "test_user"
|
||||
mock_app.config = {
|
||||
@@ -1532,20 +1718,24 @@ class TestWebDriverPlaywrightAnimationWaitOrder:
|
||||
mock_page.evaluate.side_effect = [25, 6000]
|
||||
# Empty bytes — falsy but not None; was silently passed through before the fix
|
||||
mock_take_tiled.return_value = b""
|
||||
# _get_screenshot("standalone") calls page.screenshot(full_page=True);
|
||||
# configure that return value so we can assert the fallback was reached
|
||||
# _get_screenshot("standalone") calls page.screenshot(full_page=True); it
|
||||
# must never be reached by the failure path under test.
|
||||
mock_page.screenshot.return_value = b"fallback"
|
||||
|
||||
with patch.object(WebDriverPlaywright, "auth", return_value=mock_context):
|
||||
result = WebDriverPlaywright("chrome").get_screenshot(
|
||||
"http://example.com", "standalone", mock_user
|
||||
)
|
||||
# match= keeps this assertion meaningful even when playwright
|
||||
# is not installed and PlaywrightTimeout aliases bare Exception.
|
||||
with pytest.raises(
|
||||
PlaywrightTimeout, match="Tiled screenshot failed for url"
|
||||
):
|
||||
WebDriverPlaywright("chrome").get_screenshot(
|
||||
"http://example.com", "standalone", mock_user
|
||||
)
|
||||
|
||||
assert result == b"fallback"
|
||||
# Tiled path was taken (take_tiled_screenshot was called)
|
||||
mock_take_tiled.assert_called_once()
|
||||
# Standard screenshot was called as fallback (full_page=True for "standalone")
|
||||
mock_page.screenshot.assert_called_with(full_page=True)
|
||||
# Standard screenshot must never be called as a fallback
|
||||
mock_page.screenshot.assert_not_called()
|
||||
|
||||
@patch("superset.utils.webdriver.PLAYWRIGHT_AVAILABLE", True)
|
||||
@patch("superset.utils.webdriver._browser_manager")
|
||||
|
||||
@@ -310,3 +310,66 @@ def test_save_non_editor_with_editors_field_is_rejected(
|
||||
raw_save(_view_self())
|
||||
|
||||
mock_security_manager.raise_for_editorship.assert_called_once_with(mock_orm)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Datasource.samples
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@patch("superset.views.datasource.views._", lambda s: s)
|
||||
@patch("superset.views.datasource.views.get_samples")
|
||||
@patch("superset.views.datasource.views.json_error_response")
|
||||
@patch("superset.views.datasource.views.security_manager", new_callable=MagicMock)
|
||||
def test_samples_returns_400_for_unsupported_datasource_type(
|
||||
mock_security_manager: MagicMock,
|
||||
mock_json_error_response: MagicMock,
|
||||
mock_get_samples: MagicMock,
|
||||
) -> None:
|
||||
"""Semantic views can't return raw samples — endpoint should refuse with 400."""
|
||||
from flask import Flask
|
||||
|
||||
mock_security_manager.is_guest_user.return_value = False
|
||||
mock_json_error_response.return_value = "error-response"
|
||||
|
||||
raw_samples = _get_view_func("samples")
|
||||
app = Flask(__name__)
|
||||
with app.test_request_context(
|
||||
"/datasource/samples?datasource_type=semantic_view&datasource_id=1",
|
||||
method="POST",
|
||||
json={},
|
||||
):
|
||||
result = raw_samples(_view_self())
|
||||
|
||||
assert result == "error-response"
|
||||
mock_json_error_response.assert_called_once()
|
||||
_, kwargs = mock_json_error_response.call_args
|
||||
assert kwargs.get("status") == 400
|
||||
# The bail-out must happen before any sample fetching is attempted.
|
||||
mock_get_samples.assert_not_called()
|
||||
|
||||
|
||||
@patch("superset.views.datasource.views.get_samples")
|
||||
@patch("superset.views.datasource.views.security_manager", new_callable=MagicMock)
|
||||
def test_samples_proceeds_for_supported_datasource_type(
|
||||
mock_security_manager: MagicMock,
|
||||
mock_get_samples: MagicMock,
|
||||
) -> None:
|
||||
"""A `query` datasource (supports_samples=True) bypasses the 400 short-circuit."""
|
||||
from flask import Flask
|
||||
|
||||
mock_security_manager.is_guest_user.return_value = False
|
||||
mock_get_samples.return_value = {"rows": []}
|
||||
|
||||
view = _view_self()
|
||||
raw_samples = _get_view_func("samples")
|
||||
app = Flask(__name__)
|
||||
with app.test_request_context(
|
||||
"/datasource/samples?datasource_type=query&datasource_id=1",
|
||||
method="POST",
|
||||
json={},
|
||||
):
|
||||
raw_samples(view)
|
||||
|
||||
mock_get_samples.assert_called_once()
|
||||
view.json_response.assert_called_once_with({"result": {"rows": []}})
|
||||
|
||||
Reference in New Issue
Block a user