rusackas opened a new pull request, #43500:
URL: https://github.com/apache/superset/pull/43500
### SUMMARY
`POST /api/v1/chart/` (and `PUT /api/v1/chart/<pk>`) with `datasource_type:
"saved_query"` crashes with an unhandled `AttributeError: 'SavedQuery' object
has no attribute 'name'`, surfaced to API clients as an opaque 500 `{"message":
"Fatal error"}`.
The deeper issue: `Slice.datasource` only ever resolves the `table`
relationship (`superset/models/slice.py`), so a chart pointed at any other
`datasource_type` could never actually render even if creation succeeded.
That's the case for `datasource_type: "query"` too — `Query` happens to have a
synthetic `.name` property (used for CTAS table naming, not a real display
name), so it doesn't crash, it just silently produces a permanently broken
chart instead.
`CreateChartCommand` and `UpdateChartCommand` now reject both up front with
the existing `DatasourceTypeInvalidError` (422), matching the pattern already
used for this same class of problem elsewhere in the codebase
(`explore/utils.py`, `dataset/duplicate.py`), instead of adding a new one-off
error type.
Root-caused with help from the discussion on the issue — thanks @endimonan
for the minimal repro that ruled out pivot tables as a factor and narrowed this
down.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not applicable (API error-handling behavior).
### TESTING INSTRUCTIONS
```
pytest tests/unit_tests/commands/chart/create_test.py
tests/unit_tests/commands/chart/update_test.py tests/unit_tests/commands/chart/
tests/unit_tests/charts/
```
Written test-first: each new unit test was confirmed to fail against unfixed
code (reproducing the exact `AttributeError` for `saved_query`, and the
"silently succeeds with a broken chart" behavior for `query`) before the fix
was applied, then confirmed to pass after.
Also added an integration test
(`tests/integration_tests/charts/api_tests.py::test_create_chart_from_saved_query_rejected_cleanly`)
that reproduces the original bug report's exact API call shape end-to-end —
creates a real `SavedQuery` row, then asserts `POST /api/v1/chart/` against it
returns a clean 422 instead of a 500. Couldn't run the integration suite
locally (this environment's SQLite test DB isn't migrated), so CI is the
authority on that one.
Manual repro before the fix, matching the original report:
```bash
curl -X POST "$SUPERSET_API_URL/api/v1/saved_query/" ... --data '{"db_id":
1, "label": "repro", "sql": "SELECT 1"}'
# => 201, id: N
curl -X POST "$SUPERSET_API_URL/api/v1/chart/" ... --data '{"datasource_id":
N, "datasource_type": "saved_query", "slice_name": "x", "viz_type": "table"}'
# before: 500 {"message": "Fatal error"}
# after: 422 {"message": {"datasource_type": ["Datasource type is
invalid"]}}
```
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #29697
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] 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]
