SEPURI-SAI-KRISHNA commented on code in PR #44953:
URL: https://github.com/apache/superset/pull/44953#discussion_r4278695556
##########
superset/models/helpers.py:
##########
@@ -2683,7 +2683,38 @@ def get_query_result(self, query_object: QueryObject) ->
QueryResult:
df = query_object.exec_post_processing(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
Review Comment:
Correcting myself here: I said `models/helpers.py` was back to master byte
for byte. That is wrong, and it understates the change. The diff deletes two
lines there, and the same two in `semantic_layers/models.py` along with its
now-unused `import pandas as pd`:
```
- except (TypeError, pd.errors.DataError) as ex:
- raise QueryObjectValidationError(str(ex)) from ex
```
So the broad arm you flagged is removed from both call sites rather than
left sitting there. What replaced it is `QueryObject._exec_operation`, which
catches `AttributeError`, `DataError`, `KeyError`, `TypeError` and
`ValueError`, logs with `exc_info=True`, and re-raises untouched when the
operation came from `EXTRA_PANDAS_POSTPROCESSING_OPS`, so a fault inside an
operator-registered operation still reaches the error handler as a 500.
The substance of your point is addressed; my description of where the diff
landed was not accurate.
--
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]
