Copilot commented on code in PR #40512: URL: https://github.com/apache/superset/pull/40512#discussion_r3457373229
########## superset/config.py: ########## @@ -196,6 +196,16 @@ def _try_json_readsha(filepath: str, length: int) -> str | None: SUPERSET_DASHBOARD_PERIODICAL_REFRESH_LIMIT = 0 SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE = None +# Manual dashboard refresh can stagger chart data requests across this many +# milliseconds so they do not all hit the backend at the same instant. This +# defaults to 0, which preserves the original behavior where every chart +# request fires at the same time when the user clicks the Refresh dashboard +# button. Set a positive value to opt in to staggering; the frontend then +# uses the larger of this value and the dashboard's stagger_time metadata. +# If this key is removed entirely (treated as null), the frontend falls back +# to a built-in default of 5000 ms. +SUPERSET_DASHBOARD_MANUAL_REFRESH_STAGGER_MS = 0 Review Comment: `SUPERSET_DASHBOARD_MANUAL_REFRESH_STAGGER_MS` is set to `0` by default, which keeps manual refresh fully parallel in a fresh install and won’t reduce backend request bursts unless an operator changes the config. This conflicts with the PR description’s stated default (5000ms) and the frontend hook comment that says 5000ms matches the backend default. Consider defaulting this to 5000ms and letting operators opt out by setting it to 0. ########## docs/docs/faq.mdx: ########## @@ -181,6 +181,20 @@ value in milliseconds in the JSON Metadata field: Here, the entire dashboard will refresh at once if periodic refresh is on. The stagger time of 2.5 seconds is ignored. +The manual **Refresh dashboard** button can also stagger its chart requests, controlled by the +`SUPERSET_DASHBOARD_MANUAL_REFRESH_STAGGER_MS` server config in `superset_config.py`. This defaults +to `0`, which preserves the original behavior where every chart request fires at the same time when +the button is clicked. To opt in to staggering, set a positive number of milliseconds; the window +then becomes the larger of this value and the per-dashboard `stagger_time` metadata. If the config +key is removed entirely (treated as null), the frontend falls back to a built-in default of `5000` +milliseconds. Review Comment: The FAQ describes this config as defaulting to `0` and suggests “removing the key entirely” to get a 5000ms fallback. In practice the backend always provides a value from `superset/config.py`, so “removing the key” isn’t an operator action. This section should be updated to reflect the intended default (5000ms if that’s the desired behavior) and describe `0` as the explicit opt-out; if keeping a fallback path, describe setting the config to `None`/`null` rather than “removing the key”. -- 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]
