rusackas commented on code in PR #38449:
URL: https://github.com/apache/superset/pull/38449#discussion_r3364302497
##########
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)
+
+ 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 (WebDriverException, Exception) as ex: # noqa: BLE001
Review Comment:
Fixed — simplified to `except Exception: # noqa: BLE001` (since
`WebDriverException` is a subclass) and dropped the now-unused
`WebDriverException` import. Pushed in 6ca67b0879.
##########
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)
+
+ 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 (WebDriverException, Exception) as ex: # noqa: BLE001
+ logger.exception("Error warming up cache for %s: %s", url, ex)
Review Comment:
Fixed — changed to `logger.exception("Error warming up cache for %s", url)`,
relying on the implicit `exc_info=True`. Pushed in 6ca67b0879.
##########
superset/utils/webdriver.py:
##########
@@ -740,5 +794,9 @@ def get_screenshot(self, url: str, element_name: str, user:
User) -> bytes | Non
logger.warning("exception in webdriver", exc_info=ex)
raise
finally:
Review Comment:
Fixed — removed the no-op `finally: pass` and moved the cleanup-strategy
explanation to a comment placed before the `try` block, so a reader sees it
before entering the try. 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]
