eugeneo17 opened a new pull request, #43303:
URL: https://github.com/apache/superset/pull/43303
### SUMMARY
Fixes #33615.
Charts imported as part of a dashboard ZIP are persisted with
`Slice.query_context = NULL` — the v1 importer never generates one. The Chart
Data read path requires it, so the first `GET /api/v1/chart/{pk}/data/`
returns
`400 "Chart has no query context saved. Please save the chart again."` until
each
chart is manually re-opened and saved in Explore. That workaround does not
scale
(the reporter has ~100 imported charts).
Root cause: `import_chart` (`superset/commands/chart/importers/v1/utils.py`)
copies
the exported chart config verbatim; `migrate_chart` only *rewrites* an
existing
`query_context` and never synthesizes one,a
`query_context` yields a `NULL` column, which `ChartDataRestApi.get_data`
(`superset/charts/data/api.py`) rejects with a 400.
**Fix.** `import_chart` now derives and persists a `query_context` when the
imported
chart lacks one. Design decisions:
- **Non-destructive / idempotent** — an existing `query_context` is never
overwritten (re-importing is safe).
- **Authorization-preserving** — the datasext is taken
only from the importer-resolved datasource id/type, never from the chart
`params`, so it names the same real datasource the read path authorizes (no
RLS/authz bypass).
- **Honest-fail** — charts with no derivabing to
query, or a datasource-less viz such as `markup`/`handlebars`/`divider`)
are left
`NULL` rather than given a fabricated/in
- **Resilient** — a per-chart derivation error is logged and skipped, never
aborting
the import bundle.
- The chart-data **read path is unchanged**.
A `superset charts backfill-query-context es charts
that were imported before this fix and still have a `NULL` `query_context`.
**Design note / open question for maintainontext`
mapping is per-plugin frontend logic (`buildQuery.ts`), so a pure-backend
derivation is only approximate for complex viz types. This branch also
includes an
optional full-fidelity path that runs the the backend
via an embedded V8 (`py_mini_racer`) over a generated bundle, with
byte-for-byte
parity across 29 viz types; `import_chart` uses it when available and falls
back to
the pure Python derivation otherwise. Shiptime
dependency + the JS bundle is a packaging decision that likely warrants a
SIP — I'm
happy to split it into a separate PR/SIP and keep this PR to the
dependency-free
synthesis if preferred.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED G
API behavior (no UI change):
- **Before:** import a dashboard ZIP → `GET /api/v1/chart/{pk}/data/` →
`400 {"message": "Chart has no query context saved. Please save the chart
again."}`
- **After:** same import → `GET /api/v1/chquery results,
with no manual re-save.
### TESTING INSTRUCTIONS
New/updated tests:
```bash
# Unit (helper + importer)
pytest tests/unit_tests/charts/commands/imder_test.py \
tests/unit_tests/charts/commands/importers/v1/import_test.py
# Full-fidelity parity (backend V8 output == frontend buildQuery, per viz
type)
pytest
tests/unit_tests/charts/commands/importers/v1/query_context_parity_test.py
# Integration (needs a metadata DB + example data)
pytest
tests/integration_tests/charts/commands_tests.py::TestImportChartsCommand
pytest tests/integration_tests/charts/datataApi
Verified locally: unit 27 passed; parity 29/29 (byte-identical across all
covered
viz types); TestImportChartsCommand 6/6; T existing
chart-data tests are kept unmodified as a no-regression guard). Manual:
import a
dashboard ZIP whose charts have no query_context, then GET
/api/v1/chart/{id}/data/
returns rows instead of a 400.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #33615
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in SIP-59
(https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [x] Introduces new feature or API: superset charts backfill-query-context
CLI; optional py_mini_racer runtime dependency for the full-fidelity path (see
Design note)
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
