SEPURI-SAI-KRISHNA opened a new pull request, #43225:
URL: https://github.com/apache/superset/pull/43225
<!-- PR TITLE: fix(api): restore three schema fields disabled by a stray
trailing comma -->
### SUMMARY
Three field declarations in `superset/charts/schemas.py` are wrapped in
parentheses with a trailing comma, which makes them one-element **tuples**
rather than `Field` instances:
```python
groupby = (
fields.List(
fields.String(allow_none=False, metadata={...}),
metadata={"minLength": 1},
required=True,
), # <-- this comma makes the whole thing a tuple
)
```
Marshmallow collects only `Field` instances into a schema, so an attribute
like this is dropped entirely: it never appears in the OpenAPI spec, and its
`required=True` is never applied.
The three affected declarations are each the parameter its post-processing
operation cannot run without:
| Schema | Field | Signature |
| --- | --- | --- |
| `ChartDataAggregateOptionsSchema` | `groupby` | `aggregate(df, groupby,
aggregates)` |
| `ChartDataRollingOptionsSchema` | `columns` | `rolling(df, rolling_type,
columns, ...)` |
| `ChartDataPivotOptionsSchema` | `index` | `pivot(df, index, aggregates,
...)` |
All three are parameters with no default, and the frontend operators send all
three (`pivotOperator` sends `index`, `rollingWindowOperator` sends
`columns`).
So `/swagger/v1` currently documents `pivot` without `index` and `aggregate`
without `groupby` — the one option each caller must provide.
Confirmed before and after:
```
# master
ChartDataAggregateOptionsSchema: fields=['aggregates']
ChartDataPivotOptionsSchema: fields=['aggregates', 'column_fill_value',
'columns', ...] # no 'index'
# this branch
ChartDataAggregateOptionsSchema: fields=['aggregates', 'groupby'] groupby:
required=True
ChartDataPivotOptionsSchema: fields=[..., 'index', ...] index:
required=True
```
**No runtime behaviour changes.** These schemas are documentation only —
`ChartDataPostProcessingOperationSchema.options` is a bare `fields.Dict`, so
nothing validates against them; they are registered solely for OpenAPI spec
generation. The only place any of them is `.load()`ed is a unit test for
`rolling`, whose payload does not include `columns`, and `columns` stays
optional.
I scanned the whole `superset/` tree for the pattern; these three are the
only
occurrences.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — the change is to the OpenAPI schema definitions.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/charts/test_schemas.py
```
Two tests are added:
| test | asserts |
| --- | --- |
| `test_schema_attributes_are_fields_not_tuples` | no attribute on any
schema in the module is a tuple wrapping a `Field` — a general guard against
this typo recurring anywhere |
| `test_required_post_processing_options_are_documented` | `groupby`,
`index` and `columns` are present, and the two `required=True` ones are marked
required |
Re-introducing the trailing comma on any one of the three makes both fail.
To see the spec change, start Superset, open `/swagger/v1` and compare the
`ChartDataAggregateOptions`, `ChartDataRollingOptions` and
`ChartDataPivotOptions` definitions.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
### CHECKLIST
- [ ] CI checks pass
- [x] Tests added/updated
- [ ] Documentation updated
- [x] PR title follows conventions
--
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]
