sadpandajoe commented on code in PR #43225:
URL: https://github.com/apache/superset/pull/43225#discussion_r3869710431


##########
superset/charts/schemas.py:
##########
@@ -568,17 +570,18 @@ class 
ChartDataRollingOptionsSchema(ChartDataPostProcessingOperationOptionsSchem
     Rolling operation config.
     """
 
-    columns = (
-        fields.Dict(
-            metadata={
-                "description": "columns on which to perform rolling, mapping 
source "
-                "column to target column. For instance, `{'y': 'y'}` will 
replace the "
-                "column `y` with the rolling value in `y`, while `{'y': 'y2'}` 
will add "  # noqa: E501
-                "a column `y2` based on rolling values calculated from `y`, 
leaving the "  # noqa: E501
-                "original column `y` unchanged.",
-                "example": {"weekly_rolling_sales": "sales"},
-            },
-        ),
+    columns = fields.Dict(

Review Comment:
   This now publishes `rolling.columns` as an object, but 
`PostProcessingRolling` still declares it as `string[]`. Typed clients must 
choose between the exported TypeScript contract and the OpenAPI contract; an 
array reaches `rolling()` and fails at `columns.keys()`. Should the exported 
type be updated to the string-to-string mapping this operation accepts?



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