mapledan commented on PR #42785:
URL: https://github.com/apache/superset/pull/42785#issuecomment-5717118424
Follow-up pushed in cd17f3ec74, from bito's round and a re-read of it.
- `get_undefined_parameters` is now called inside the `TemplateError` catch,
as
it is on the execution path. A parameter whose *value* carried malformed
Jinja escaped `run()` as a raw `TemplateSyntaxError` — a 500 where Run
returns a structured 400.
- The reason string and its suggestion moved to `jinja_context`
(`undefined_parameters_message`, `PARAMETER_MISSING_ERR`) and both paths
use
them, so the two reports cannot drift. The estimate response now carries
the
suggestion Run has always sent.
- `get_undefined_parameters` no longer swallows a parse failure. That was a
behavior change to Run I had not accounted for: `raise_for_access` returns
early for a caller with database access, so swallowing the error could have
let unparseable SQL past `_validate` and into execution. `SELECT * FROM
{{ tbl }}` is a parse error again on both paths — I would rather this PR
not
move Run's failure stage at all.
The regression test for the first item is built on a real template processor
rather than a mock. That is the reason none of this was caught here: the
command's tests mock `get_template_processor`, so the code that raises never
ran, and the processor's own tests never go through `run()`.
--
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]
