SEPURI-SAI-KRISHNA commented on PR #44953: URL: https://github.com/apache/superset/pull/44953#issuecomment-5971603866
Fixed the `KeyError` arm so it logs with `exc_info=True`, like the branch below it. Both reviews land on the same gap: a custom op registered through `EXTRA_PANDAS_POSTPROCESSING_OPS` can raise `KeyError` from its own internals, where the "column or level" wording is a guess. `exec_post_processing` does dispatch those ops, so the path is real. Naming the operation in the message would need the name from inside that loop, which means moving the translation into `QueryObject`, so I left it and kept the traceback instead. One correction: `cum`, `diff` and `select` all have `validate_column_args`. Of those four only `rank` lacks it. The point still holds via `rank`, `boxplot`, `histogram`, `flatten`, `resample`, `prophet` and geography, and because the decorator only checks the args it is given (`aggregate` checks `groupby`, not the columns inside `aggregates`). Keeping the catch broad. Narrowing to pandas errors would drop the numpy `ValueError` and the `AttributeError` cases, 28 of the 38 sites here. `TypeError` was already caught by #44463 and #44502 and is no narrower than `ValueError`. Leaving the duplicated test setup. Deduping means editing the sibling test, and `helpers_test.py` is also touched by the open #44897. -- 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]
