SEPURI-SAI-KRISHNA opened a new pull request, #43710:
URL: https://github.com/apache/superset/pull/43710
### SUMMARY
Fixes #43356.
`ChartDataProphetOptionsSchema.time_grain` validates against
`get_time_grain_choices()`, which merges `builtin_time_grains` with the
operator-configured `TIME_GRAIN_ADDONS`. `prophet()` then resolves the grain
through `PROPHET_TIME_GRAIN_MAP`, a static map that cannot contain an
operator's arbitrary addon key. So a configured custom grain passes schema
validation and then fails at the lookup, the API advertises a forecast it
cannot serve.
Reproducing on `master` with `TIME_GRAIN_ADDONS = {"PT7M": "7 minute"}`:
```
PT7M is a built-in mapped grain? False
schema.load('PT7M') : ACCEPTED
prophet('PT7M') : InvalidPostProcessingError -> Unsupported time grain:
PT7M
```
The addon keys are arbitrary ISO-8601 durations, so the static map cannot
simply be extended to cover them, and converting them to pandas frequencies
at
runtime is not total, `P0.25Y` being the obvious case. That leaves the other
option the issue identifies: publish only what the operation can resolve.
This adds `get_prophet_time_grain_choices()`, returning the keys of
`PROPHET_TIME_GRAIN_MAP`, and points the Prophet field's validator at it.
`get_time_grain_choices()` is deliberately left alone, it is shared with
`time_grain_sqla` on `ChartDataExtrasSchema`, where addon grains are
legitimately supported, and narrowing it there would be a regression.
**This widens the published set as well as narrowing it.** `PT0.5H`
(`HALF_HOUR`) and `P0.25Y` (`QUARTER_YEAR`) are in the map but not in
`builtin_time_grains`, so they were previously rejected by the schema despite
resolving fine in `prophet()`. They are now accepted. Net effect on a default
deployment is 19 advertised grains -> 21, with every one of them resolvable.
No working request changes behaviour: the grains this removes from the spec
never produced a forecast.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A, backend-only fix.
### TESTING INSTRUCTIONS
```bash
pytest tests/unit_tests/charts/test_schemas.py -q
pytest tests/unit_tests/pandas_postprocessing/test_prophet.py -q
```
Three tests are added:
- `test_prophet_schema_advertises_only_resolvable_grains` asserts the field's
own `OneOf` choices equal the map's keys. It reads the validator rather
than
the helper, so re-pointing the field back at `get_time_grain_choices()`
fails
here rather than silently reintroducing the bug.
- `test_prophet_rejects_addon_grain_accepted_by_time_grain_sqla` configures
`TIME_GRAIN_ADDONS = {"PT7M": "7 minute"}` and asserts the addon is still
advertised by the shared helper while the Prophet schema rejects it. This
is
the reported bug, and it pins the "do not narrow the shared helper"
constraint.
- `test_prophet_accepts_every_mapped_grain` covers the five grains that now
load and resolve, including the two newly advertised aliases.
The three pre-existing `time_grain` assertions are unchanged and still pass —
`P1D` remains valid and `invalid_grain` remains rejected.
### ADDITIONAL INFORMATION
- [x] Has associated issue: #43356
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in SIP-59)
- [ ] 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
Note for reviewers: #43206 also edits `ChartDataProphetOptionsSchema`, about
fifty lines below this change (`monthly_seasonality` -> `daily_seasonality`,
plus `index`). Separate hunks, so the two should merge cleanly, but whichever
lands second may want a rebase.
--
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]
