rusackas commented on code in PR #43585:
URL: https://github.com/apache/superset/pull/43585#discussion_r3890283937
##########
tests/unit_tests/charts/test_schemas.py:
##########
@@ -310,21 +310,30 @@ def
test_chart_data_query_object_schema_deprecated_fields_renamed(
@pytest.mark.parametrize(
"app",
- [{"TIME_GRAIN_ADDONS": {"PT10M": "10 minutes"}}],
+ [{"TIME_GRAIN_ADDONS": {"PT7M": "7 minutes"}}],
indirect=True,
)
def test_time_grain_validation_with_config_addons(app_context: None) -> None:
- """Test that validation includes TIME_GRAIN_ADDONS from config"""
- schema = ChartDataProphetOptionsSchema()
+ """
+ Test that custom TIME_GRAIN_ADDONS are accepted by ChartDataExtrasSchema
(SQLA)
+ but rejected by ChartDataProphetOptionsSchema (which only supports mapped
Prophet grains).
+ """
+ # Custom addon grain is valid for SQLA time grain
+ extras_schema = ChartDataExtrasSchema()
+ extras_result = extras_schema.load({"time_grain_sqla": "PT7M"})
+ assert extras_result["time_grain_sqla"] == "PT7M"
Review Comment:
Traced this one, doesnt hold up against the current code.
`validate_time_grain_sqla` is a plain function reference, marshmallow calls it
at validation time, not at import time, so it picks up `TIME_GRAIN_ADDONS` from
the fixture fine. Confirmed with an isolated repro of the eager-vs-lazy
pattern. Resolving.
##########
superset/charts/schemas.py:
##########
@@ -757,7 +758,7 @@ class
ChartDataProphetOptionsSchema(ChartDataPostProcessingOperationOptionsSchem
"[ISO 8601](https://en.wikipedia.org/wiki/ISO_8601#Durations)
durations.",
"example": "P1D",
},
- validate=validate.OneOf(choices=get_time_grain_choices()),
+ validate=validate.OneOf(choices=tuple(PROPHET_TIME_GRAIN_MAP.keys())),
Review Comment:
Agreed, traced this myself and it checks out, left the detail in my review
comment above.
--
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]
