SEPURI-SAI-KRISHNA commented on code in PR #44953:
URL: https://github.com/apache/superset/pull/44953#discussion_r4278775007
##########
superset/semantic_layers/models.py:
##########
@@ -376,7 +376,38 @@ def get_query_result(self, query_object: QueryObject) ->
QueryResult:
result.df = query_object.exec_post_processing(result.df)
except InvalidPostProcessingError as ex:
raise QueryObjectValidationError(ex.message) from ex
- except (TypeError, pd.errors.DataError) as ex:
+ # A post-processing operation is driven entirely by the request's
+ # `options` dict, which `ChartDataPostProcessingOperationSchema`
+ # accepts as an untyped `fields.Dict`. A malformed option therefore
+ # reaches pandas and surfaces as whatever pandas raises, so these
+ # are bad-request failures, not server faults. ImportError is
+ # deliberately excluded: a missing optional dependency (scipy, for
+ # a `rolling` win_type) is a deployment matter, not a bad request.
+ except KeyError as ex:
+ # Every KeyError an operation raises names a column or
+ # MultiIndex level the options asked for and the result does
+ # not have. `str(KeyError)` is only the repr'd key, which alone
+ # reads as a bare quoted string.
+ raise QueryObjectValidationError(
+ _(
+ "Post-processing references a column or level that is "
+ "not in the query result: %(name)s",
+ name=ex.args[0] if ex.args else ex,
+ )
+ ) from ex
+ except (
+ TypeError,
+ ValueError,
+ AttributeError,
+ pd.errors.DataError,
+ ) as ex:
+ # These types are broad enough to also cover a genuine fault in
+ # an operation, and the caller turns this into a message on the
+ # response without logging it, so keep the traceback.
+ logger.warning(
+ "Post-processing failed and was reported as a bad request",
+ exc_info=True,
+ )
raise QueryObjectValidationError(str(ex)) from ex
Review Comment:
This one is fixed rather than argued, and you had it right.
The conversion moved out of the two call sites into
`QueryObject._exec_operation`, which re-raises untouched when the operation is
not built-in:
```python
except (AttributeError, DataError, KeyError, TypeError, ValueError) as ex:
if not is_builtin:
raise
```
The `except (TypeError, pd.errors.DataError)` arm that sat next to the
`InvalidPostProcessingError` handler here and in `models/helpers.py` is
removed, so nothing downgrades an operator-owned fault to a 400 any more.
`semantic_layers/models.py` loses its `import pandas as pd` with it.
Three tests pin it, one per layer, each registering an operation through
`EXTRA_PANDAS_POSTPROCESSING_OPS` that raises and asserting it propagates:
- `test_exec_post_processing_leaves_a_custom_operation_fault_unwrapped`
- `test_get_query_result_leaves_a_custom_operation_fault_unwrapped`
-
`test_semantic_view_get_query_result_leaves_a_custom_operation_fault_unwrapped`
Putting either handler back fails them.
--
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]
