aminghadersohi commented on code in PR #45359:
URL: https://github.com/apache/superset/pull/45359#discussion_r4289293215
##########
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:
Nit: `datasource.metrics` and `.columns` are materialized on every request,
including cache hits and empty selections where no name can fail. On
`SqlaTable` these are lazy relationships, so the cache-hit path pays two extra
queries it did not before.
##########
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:
An ad-hoc dict, `None` or an int in these lists raises `TypeError` and
returns 500 where master returned 200 — and 400 vs 500 flips on whether the
datasource has members. `DatasourceQuerySchema` accepts ad-hoc dicts;
`validate_query_names` filters non-str for this.
```suggestion
selection_errors: list[str] = validate_names(
[name for name in selected_metrics if isinstance(name, str)],
{metric.metric_name for metric in datasource.metrics},
"metric",
) + validate_names(
[name for name in selected_dimensions if isinstance(name, str)],
{dimension.column_name for dimension in datasource.columns},
"dimension",
)
```
--
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]
