rusackas opened a new pull request, #44549:
URL: https://github.com/apache/superset/pull/44549
### SUMMARY
`QueryContextFactory.create()` requires `datasource` and `queries` as
keyword-only arguments, but `ChartPostSchema`/`ChartPutSchema` only checked
that a chart's `query_context` was well-formed JSON, not that it carried those
keys. A chart saved with an incomplete `query_context` (a buggy integration,
hand-edited metadata, a partial migration) fails every subsequent read with a
raw `TypeError`, which surfaces to users as a `"Error loading chart
datasources"` toast (422) with no indication of the actual cause.
Two earlier attempts at this fix (#35798, #36076) went stale without
merging. This is a fresh implementation, informed by the review issues those
hit along the way — a required-fields check that could be bypassed by a falsy
*parsed* JSON value (`"false"`, `"0"`, `"null"` as `query_context`), and
duplicated JSON-decode logic alongside the existing `validate_json`.
**Changes:**
- `superset/utils/schema.py`: `validate_query_context_metadata()`, composed
on top of the existing `validate_json` parsing rather than duplicating it,
checks the parsed payload is a JSON object with non-empty `datasource` and
`queries`. Wired into `ChartPostSchema`/`ChartPutSchema` in place of the plain
JSON-syntax check.
- `superset/models/slice.py`: `Slice.get_query_context()` now also treats a
stored `query_context` missing either field as unreadable (returns `None`, logs
an error) rather than letting the `TypeError` propagate — the same
"honest-fail" pattern it already used for malformed JSON just above. This
matters because the schema fix alone only prevents *new* bad saves; it can't
retroactively fix rows saved before this validation existed (exactly the
situation the issue reporter is in).
Unrelated but touched in passing: `slice.py` had a pre-existing ruff finding
(`S704`, unsafe `Markup` f-string interpolation) on a line this diff's context
otherwise includes — fixed via `Markup(...).format(...)`, which auto-escapes
non-`Markup` arguments; verified byte-identical output for the existing case.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — backend validation change, no UI.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/utils/test_schema.py
pytest tests/unit_tests/models/slice_test.py -k query_context
pytest tests/unit_tests/charts/test_schemas.py -k query_context
```
Manual: `POST /api/v1/chart/` with `query_context` set to `'{"a": 1}'` (or
any object missing `datasource`/`queries`) now returns a 422 validation error
instead of saving. A chart whose `query_context` predates this fix (missing
those keys) no longer throws on read — `Slice.get_query_context()` returns
`None` and logs the reason instead.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #35774
- [ ] 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
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
