mikebridge opened a new pull request, #43885: URL: https://github.com/apache/superset/pull/43885
### SUMMARY `set_related_perm` — the `before_insert` / `before_update` listener on `Slice` that denormalizes the datasource's permission columns onto the chart — began with an **unguarded** lookup: ```python src_class = DatasourceDAO.sources[target.datasource_type] ``` `DatasourceDAO.sources` only maps `table`, `query`, `saved_query`, and `semantic_view`. Any other `datasource_type` — a legacy connector (e.g. the old `druid`), a typo, or an extension type core does not know yet — raises a `KeyError` **inside the flush**. Because this fires on every insert and update, it 500s *every* save of such a chart, including saves that never touch datasource fields. And since the lookup runs *before* the `if target.datasource_id` guard, even an unknown-type chart with no `datasource_id` crashes. This guards the lookup with `.get()`: an unknown type now logs a warning and returns, leaving the perm columns untouched, so the transaction proceeds. The read side already treats an unsupported datasource type as inaccessible (fail closed); the write-side perm denormalization now degrades the same way instead of aborting the flush. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A — backend-only. Behavior is covered by unit tests. **Before:** saving a chart whose `datasource_type` is not one of `table`/`query`/`saved_query`/`semantic_view` raises `KeyError` in the `before_insert`/`before_update` listener and the flush aborts (HTTP 500). **After:** the save succeeds; the perm columns are left unset and a warning is logged naming the unknown type. ### TESTING INSTRUCTIONS `pytest tests/unit_tests/models/slice_test.py -k set_related_perm` — two new tests assert that a slice with an unknown `datasource_type` (with and without a `datasource_id`) is saved without raising. Both fail on `master` with `KeyError` and pass with this change. ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [ ] 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]
