SEPURI-SAI-KRISHNA commented on issue #44952:
URL: https://github.com/apache/superset/issues/44952#issuecomment-5971977464

   Follow-up on the root cause, since #44953 only changes the status code and 
leaves the bad option reaching pandas.
   
   `options` stays an untyped `fields.Dict`. Of the 19 operations, 8 have no 
options schema at all (`compare`, `cum`, `diff`, `flatten`, `histogram`, 
`rank`, `rename`, `resample`), and the 11 that do exist are only listed in 
`CHART_SCHEMAS` for the OpenAPI spec, so they validate nothing.
   
   There is already a TODO for this at `superset/charts/schemas.py:2137`:
   
   > These should optimally be included in the QueryContext schema as an 
`anyOf` in ChartDataPostProcessingOperation.options, but since `anyOf` is not 
[supported] by Marshmallow<3, this is not currently possible.
   
   That blocker looks gone. The pin is `marshmallow==4.3.1`, 
`marshmallow-union` is already a dependency, and `Union` is already imported in 
the same file and used for `ChartDataDatasourceSchema.id`.
   
   Two ways to wire it:
   
   1. An untagged `Union` of the options schemas, which is what the TODO's 
`anyOf` describes.
   2. Dispatch on `operation` in a `@validates_schema` on the parent, so a bad 
option names the operation it belongs to. Better messages, which is the point 
of doing this at all.
   
   I would rather ask before writing it, because it is a breaking change: 
payloads that are malformed but tolerated today would start getting a 400. The 
part that needs a maintainer call is which of these you want:
   
   - reject outright
   - log a deprecation warning for one release, then enforce
   - enforce behind a feature flag
   
   Happy to implement whichever, including the 8 missing schemas and 
per-operation tests. I can open a separate issue for it if there is appetite.
   


-- 
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]

Reply via email to