eschutho opened a new pull request, #41056:
URL: https://github.com/apache/superset/pull/41056
### What was the warning?
Every chart data request (`POST /api/v1/chart/data`) from a chart with
`groupby`
in its saved query context emitted a `logger.warning` per request:
```
The field `groupby` is deprecated, please use `columns` instead.
```
This fires in `QueryObject._rename_deprecated_fields`
(superset/common/query_object.py:224)
whenever `groupby` lands in `**kwargs`. The same path exists for
`granularity_sqla`,
`timeseries_limit`, and `timeseries_limit_metric`.
Observed in production on the `wealthsimple-aws-mpc` cluster every time a
chart using the legacy `groupby` key is loaded.
### What was changed?
Added a `@post_load` hook (`rename_deprecated_fields`) to
`ChartDataQueryObjectSchema`
that renames all four deprecated query object fields to their canonical
names at the
API schema boundary:
| Deprecated | Canonical |
|---|---|
| `groupby` | `columns` |
| `granularity_sqla` | `granularity` |
| `timeseries_limit` | `series_limit` |
| `timeseries_limit_metric` | `series_limit_metric` |
This matches the existing behavior of `_rename_deprecated_fields` exactly:
if the old name is truthy, it becomes the new name; falsy values (empty list,
`None`, `0`) are discarded and the existing canonical value is preserved.
### No behavior change
`QueryObject` still receives the same canonical field names and values as
before. The only difference is the rename now happens at the schema layer
(before `QueryObjectFactory.create()`) instead of inside
`QueryObject.__init__`.
The `_rename_deprecated_fields` method remains as a defensive fallback for
any non-schema callers.
### Test plan
- [ ] New unit tests in `tests/unit_tests/charts/test_schemas.py`:
- `groupby` alone → becomes `columns`
- `groupby` overwrites existing `columns` (matches existing behavior)
- empty `groupby` (`[]`) → `columns` preserved
- `null` `groupby` (valid per `allow_none=True`) → `columns` preserved
- no `groupby` → `columns` unchanged
- `granularity_sqla` → `granularity`
- `timeseries_limit` → `series_limit`
- `timeseries_limit_metric` → `series_limit_metric`
- [ ] All pre-commit hooks pass (mypy, ruff, pylint, blacklist)
- [ ] Confirm `logger.warning` no longer fires for `groupby`-bearing chart
data requests in production
--
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]
