bito-code-review[bot] commented on PR #40568:
URL: https://github.com/apache/superset/pull/40568#issuecomment-4609150089
<!-- Bito Reply -->
The flagged issue is correct. Raising
`SupersetDisallowedSQLFunctionException` or
`SupersetDisallowedSQLTableException` directly causes an HTTP 500 error because
these exceptions do not specify a 4xx status code. To preserve the API
contract, these should be wrapped in `QueryObjectValidationError` (which maps
to 400) or a similar exception that triggers a client-side error response.
To resolve this, you can catch these exceptions in `_process_sql_expression`
and re-raise them as `QueryObjectValidationError`:
```python
if disallowed_functions and parsed.check_functions_present(
disallowed_functions
):
raise QueryObjectValidationError(f"Disallowed functions:
{disallowed_functions}")
if disallowed_tables and
parsed.check_tables_present(disallowed_tables):
# ... (table logic) ...
raise QueryObjectValidationError(f"Disallowed tables:
{found_tables or disallowed_tables}")
```
There are no other comments in this PR to address.
**superset/models/helpers.py**
```
if disallowed_functions and parsed.check_functions_present(
disallowed_functions
):
raise QueryObjectValidationError(f"Disallowed functions:
{disallowed_functions}")
if disallowed_tables and
parsed.check_tables_present(disallowed_tables):
# ... (table logic) ...
raise QueryObjectValidationError(f"Disallowed tables:
{found_tables or disallowed_tables}")
```
--
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]