rebenitez1802 commented on code in PR #45742:
URL: https://github.com/apache/superset/pull/45742#discussion_r4380863284
##########
superset/connectors/sqla/models.py:
##########
@@ -2377,7 +2377,7 @@ def _render_adhoc_expression_for_metadata_lookup(
return sql_expression
try:
return template_processor.process_template(sql_expression)
- except SupersetSyntaxErrorException as ex:
+ except (TemplateError, SupersetSyntaxErrorException) as ex:
Review Comment:
🟡 **Medium: same 500 still reachable via adhoc metrics / metric orderby (OK
as a follow-up)**
This fixes adhoc columns, and adhoc filters whose `col` is an adhoc SQL
column. A `{{ foo.bar }}` in an **adhoc SQL metric**, or in an orderby on one,
still returns a 500 from `/api/v1/chart/data`. Two places are affected:
- `SqlaTable.adhoc_metric_to_sqla` (`models.py:2349-2357`) only catches
`SupersetSecurityException`.
- `_process_metric_orderby_expression` in `superset/models/helpers.py`
(~L5110) has no template-error handling.
That code is outside this diff, so I can't attach an applyable suggestion.
Something like this in `adhoc_metric_to_sqla` would close the metric case. It
reuses the existing "metric expression" msgid, so `.pot` won't drift:
```python
if not processed:
try:
expression = self._process_metric_select_expression(
expression=expression,
engine=self.database.backend,
schema=self.schema,
template_processor=template_processor,
)
except SupersetSecurityException as ex:
raise QueryObjectValidationError(ex.message) from ex
except (TemplateError, SupersetSyntaxErrorException) as ex:
raise QueryObjectValidationError(
_(
"Error in jinja expression in metric expression:
%(msg)s",
msg=str(ex),
)
) from ex
```
The orderby path in `helpers.py` would need the same wrapping. Not blocking:
happy to see this in this PR or in a follow-up.
--
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]
