SEPURI-SAI-KRISHNA commented on code in PR #44953:
URL: https://github.com/apache/superset/pull/44953#discussion_r4218240743
##########
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:
Moot as of the restructure: this handler no longer exists.
`models/helpers.py` is back to master byte for byte, and the wrapping moved
into `QueryObject.exec_post_processing`'s loop, where it logs with
`exc_info=True` before raising. Both arms log now, which was the gap you
pointed at.
##########
superset/models/helpers.py:
##########
@@ -2683,7 +2683,45 @@ 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 a built-in 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. An operation registered
+ # through EXTRA_PANDAS_POSTPROCESSING_OPS could raise KeyError
+ # from its own internals, where that wording would be a guess,
+ # which is the other reason to keep the traceback.
+ logger.warning(
+ "Post-processing failed and was reported as a bad request",
+ exc_info=True,
+ )
+ 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:
Review Comment:
Addressed, and you were right to push on it. The guard moved into
`QueryObject.exec_post_processing`'s loop, which knows whether an operation is
built-in or registered through `EXTRA_PANDAS_POSTPROCESSING_OPS`. Only built-in
operations are wrapped; a fault inside a custom operation propagates and stays
a 500, with a test for it. The call-site handlers that #44463 and #44502 added
are removed too, so `TypeError` and `DataError` from a custom operation are no
longer converted either. This file is back to master byte for byte.
--
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]
