rusackas commented on code in PR #38449:
URL: https://github.com/apache/superset/pull/38449#discussion_r3364301585
##########
superset/tasks/cache.py:
##########
@@ -307,25 +234,39 @@ def cache_warmup(
logger.exception(message)
return message
- results: dict[str, list[str]] = {"scheduled": [], "errors": []}
- for task in strategy.get_tasks():
- username = task["username"]
- payload = json.dumps(task["payload"])
- if username:
+ results: dict[str, list[str]] = {"success": [], "errors": []}
+
+ warmup_username = current_app.config.get("SUPERSET_CACHE_WARMUP_USER")
+ if not warmup_username:
+ message = (
+ "SUPERSET_CACHE_WARMUP_USER is not configured. Set it to a
dedicated "
+ "least-privilege user with access to the dashboards you want
warmed up."
+ )
+ logger.error(message)
+ return message
+
+ user = security_manager.find_user(username=warmup_username)
+ if not user:
+ message = (
+ f"Cache warmup user '{warmup_username}' not found. Please
configure "
+ "SUPERSET_CACHE_WARMUP_USER with a valid username."
+ )
+ logger.error(message)
+ return message
+
+ wd = WebDriverSelenium(current_app.config["WEBDRIVER_TYPE"], user=user)
Review Comment:
Good call — added unit test coverage for the new path in
`tests/unit_tests/tasks/test_cache.py`: missing-config fast-fail,
user-not-found fast-fail, the happy path (verifies `get_screenshot` is called
per URL and `destroy()` runs in `finally`), error accumulation, and the
unknown-strategy case. Pushed in 6ca67b0879.
##########
superset/config.py:
##########
@@ -1070,6 +1070,11 @@ class D3TimeFormat(TypedDict, total=False):
}
THUMBNAIL_ERROR_CACHE_TTL = int(timedelta(days=1).total_seconds())
+# Cache warmup user — must be set explicitly before enabling the cache-warmup
+# Celery task. Intentionally defaults to None so operators pick a dedicated
+# least-privilege user rather than inadvertently running warmup as "admin".
+SUPERSET_CACHE_WARMUP_USER: str | None = None
Review Comment:
Done — added an `UPDATING.md` entry under "Next" explaining that the
cache-warmup path now authenticates via `SUPERSET_CACHE_WARMUP_USER` and no
longer consults `CACHE_WARMUP_EXECUTORS`, with the migration step for operators
who relied on the old flag. Also added a note next to `CACHE_WARMUP_EXECUTORS`
in `config.py` clarifying it is no longer used by the `cache-warmup` task.
Pushed in 6ca67b0879.
--
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]
