mikebridge commented on code in PR #45359:
URL: https://github.com/apache/superset/pull/45359#discussion_r4289580549
##########
superset/datasource/api.py:
##########
@@ -651,6 +652,18 @@ def compatible(self, datasource_type: str, datasource_id:
int) -> FlaskResponse:
selected_metrics = body.get("selected_metrics", [])
selected_dimensions = body.get("selected_dimensions", [])
+ selection_errors: list[str] = validate_names(
+ selected_metrics,
+ {metric.metric_name for metric in datasource.metrics},
+ "metric",
+ ) + validate_names(
+ selected_dimensions,
+ {dimension.column_name for dimension in datasource.columns},
+ "dimension",
+ )
Review Comment:
Good catch — `80633a0b84` filters non-string list entries before validation,
cache-key sorting, and compatibility dispatch, and returns 400 for non-list
fields. Would it be worth keeping the normalization at this boundary? Filtering
only inside `validate_names` would leave later mixed-type failures;
`test_compatible_ignores_non_name_selections` and
`test_compatible_rejects_non_list_selection_fields` were red before and pass
now.
##########
superset/datasource/api.py:
##########
@@ -651,6 +652,18 @@ def compatible(self, datasource_type: str, datasource_id:
int) -> FlaskResponse:
selected_metrics = body.get("selected_metrics", [])
selected_dimensions = body.get("selected_dimensions", [])
+ selection_errors: list[str] = validate_names(
+ selected_metrics,
+ {metric.metric_name for metric in datasource.metrics},
Review Comment:
Good catch — `80633a0b84` builds each name set only when that selection has
string names; `test_compatible_empty_selection_skips_metadata_on_cache_hit` was
red before and now verifies no metadata access on an empty cached request.
Could we keep validation before cache hits for named selections, so a renamed
member cannot use an older cached success?
`test_compatible_rejects_unknown_selection_before_cache` covers that ordering.
--
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]
