rusackas commented on code in PR #40640:
URL: https://github.com/apache/superset/pull/40640#discussion_r3347408740
##########
superset/charts/schemas.py:
##########
@@ -62,6 +62,34 @@ def get_time_grain_choices() -> Any:
]
+# Fallback upper bound for the number of Prophet forecast periods when the
+# application config cannot be read (for example, outside of an app context).
+DEFAULT_MAX_PROPHET_PERIODS = 10000
+
+
+def get_max_prophet_periods() -> int:
+ """Get the configured upper bound for Prophet forecast periods."""
+ try:
+ return current_app.config.get(
+ "MAX_PROPHET_PERIODS", DEFAULT_MAX_PROPHET_PERIODS
+ )
Review Comment:
Done — `get_max_prophet_periods()` now coerces the config value to `int` (so
a string override like `os.getenv(...)` no longer triggers a `TypeError` in
`Range`) and falls back to the default bound for invalid or non-positive
values. Added unit tests covering the string and invalid cases.
##########
superset/utils/schema.py:
##########
@@ -51,3 +54,26 @@ def validate_json(value: Union[bytes, bytearray, str]) ->
None:
json.validate_json(value)
except json.JSONDecodeError as ex:
raise ValidationError("JSON not valid") from ex
+
+
+def validate_external_url(value: Optional[str]) -> None:
+ """
+ Validator for externally managed object URLs.
+
+ Restricts the accepted URL schemes to ``http`` and ``https`` so that
+ other schemes (for example ``javascript:``, ``data:`` or ``vbscript:``)
+ cannot be stored and later rendered by clients. Empty values are allowed
+ since the field is optional.
+
+ :param value: the URL to validate
+ :raises ValidationError: if the value uses a disallowed scheme
+ """
+ if not value:
+ return
+
+ scheme = urlparse(value).scheme.lower()
+ if scheme not in ALLOWED_URL_SCHEMES:
+ raise ValidationError(
+ "URL must use one of the following schemes: "
+ f"{', '.join(sorted(ALLOWED_URL_SCHEMES))}."
Review Comment:
Done — `validate_external_url` now also requires a non-empty `netloc`, so
hostless values like `https:foo` are rejected rather than stored. Added
parametrized tests for the scheme-only cases.
##########
superset/charts/schemas.py:
##########
@@ -62,6 +62,34 @@ def get_time_grain_choices() -> Any:
]
+# Fallback upper bound for the number of Prophet forecast periods when the
+# application config cannot be read (for example, outside of an app context).
+DEFAULT_MAX_PROPHET_PERIODS = 10000
+
+
+def get_max_prophet_periods() -> int:
+ """Get the configured upper bound for Prophet forecast periods."""
+ try:
+ return current_app.config.get(
+ "MAX_PROPHET_PERIODS", DEFAULT_MAX_PROPHET_PERIODS
+ )
+ except RuntimeError:
+ # Outside app context, fall back to the default bound
+ return DEFAULT_MAX_PROPHET_PERIODS
+
+
+def validate_prophet_periods(value: int) -> None:
+ """Ensure the number of Prophet forecast periods stays within bounds."""
+ Range(
+ min=1,
+ max=get_max_prophet_periods(),
+ error=_(
+ "`periods` must be between 1 and %(max)s",
+ max=get_max_prophet_periods(),
+ ),
+ )(value)
Review Comment:
Done — `validate_prophet_periods` now captures `get_max_prophet_periods()`
once and reuses it for both the `Range` max and the error message, so the two
can no longer diverge.
--
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]
