bito-code-review[bot] commented on PR #43885:
URL: https://github.com/apache/superset/pull/43885#issuecomment-5733996964
<!-- Bito Reply -->
Rejecting an explicit `null` datasource type at the update boundary is a
viable approach to prevent charts from entering an unresolvable state. The
current implementation in `superset/models/slice.py` handles unknown types by
clearing permissions (failing closed), which is a robust safety measure, but
adding an explicit validation check would provide a cleaner failure mode before
the database flush occurs.
To implement this, you could add a validation check in the
`UpdateChartCommand` or the model's `before_update` listener to raise a
`ValidationError` if `datasource_type` is explicitly set to `null`. The
existing regression tests in `tests/unit_tests/models/slice_test.py` already
demonstrate the "fail closed" behavior for unknown types; you can extend these
by adding a test case that asserts a `null` type triggers a validation error
rather than proceeding to the `set_related_perm` logic.
**superset/models/slice.py**
```
src_class = DatasourceDAO.sources.get(target.datasource_type)
if target.datasource_type is None:
raise ValidationError("datasource_type cannot be null")
if src_class is None:
# ... existing fail-closed logic ...
```
--
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]
