rusackas commented on code in PR #38449:
URL: https://github.com/apache/superset/pull/38449#discussion_r3365401394
##########
superset/tasks/cache.py:
##########
@@ -307,25 +233,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)
+
+ try:
+ for url in strategy.get_urls():
try:
- user = security_manager.get_user_by_username(username)
- cookies = MachineAuthProvider.get_auth_cookies(user)
- headers = {
- "Cookie": "session=%s" % cookies.get("session", ""),
- "Content-Type": "application/json",
- }
- logger.info("Scheduling %s", payload)
- fetch_url.delay(payload, headers)
- results["scheduled"].append(payload)
- except SchedulingError:
- logger.exception("Error scheduling fetch_url for payload: %s",
payload)
- results["errors"].append(payload)
- else:
- logger.warning("Executor not found for %s", payload)
+ logger.info("Fetching %s", url)
+ wd.get_screenshot(url, "grid-container")
+ results["success"].append(url)
+ except Exception: # noqa: BLE001
+ logger.exception("Error warming up cache for %s", url)
Review Comment:
This suggestion is invalid: `WebDriverException` is a subclass of
`Exception`, so `except (WebDriverException, Exception)` is fully redundant —
the bare `Exception` clause already catches every `WebDriverException`. This
was exactly aminghadersohi's MEDIUM review note, which I addressed by
simplifying to `except Exception: # noqa: BLE001`. The full traceback
(including the Selenium-specific exception type and message) is already
captured by `logger.exception`, so no diagnostic detail is lost. The other
files mentioned catch `WebDriverException` separately only because they also
have distinct `except Exception` branches with different handling; that
distinction does not exist here. Leaving the simplified form.
--
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]
