bito-code-review[bot] commented on code in PR #35946:
URL: https://github.com/apache/superset/pull/35946#discussion_r2485328834
##########
superset-frontend/webpack.config.js:
##########
@@ -156,8 +158,29 @@ if (!isDevMode) {
}),
);
- // Runs type checking on a separate process to speed up the build
- plugins.push(new ForkTsCheckerWebpackPlugin());
+ plugins.push(
+ new ForkTsCheckerWebpackPlugin({
+ async: !isDevMode,
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Faulty ForkTsCheckerWebpackPlugin async setting</b></div>
<div id="fix">
The `async` option for `ForkTsCheckerWebpackPlugin` is incorrectly set to
`!isDevMode`, which enables asynchronous type checking in production mode. This
can allow production builds to complete before TypeScript errors are detected,
potentially deploying faulty code. Change it to `async: isDevMode` to ensure
synchronous checking in production, blocking builds with errors, while keeping
async in development for faster feedback. This affects the build process by
ensuring type safety in production deployments.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
async: isDevMode,
````
</div>
</details>
</div>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/35946#issuecomment-3478922652>#bf792c</a></i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/tasks/cache.py:
##########
@@ -307,25 +239,20 @@ 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:
- try:
- user = security_manager.get_user_by_username(username)
- cookies = MachineAuthProvider.get_auth_cookies(user)
- headers = {
- "Cookie": f"session={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.warn("Executor not found for %s", payload)
+ results: dict[str, list[str]] = {"success": [], "errors": []}
+
+ user = security_manager.find_user(
+ username=current_app.config["SUPERSET_CACHE_WARMUP_USER"]
+ )
+ wd = WebDriverSelenium(current_app.config["WEBDRIVER_TYPE"], user=user)
+
+ for url in strategy.get_urls():
+ try:
+ logger.info("Fetching %s", url)
+ wd.get_screenshot(url, "grid-container")
+ results["success"].append(url)
+ except URLError:
+ logger.exception("Error warming up cache!")
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Incomplete exception handling in cache warmup</b></div>
<div id="fix">
The exception handler in the cache_warmup function only catches URLError,
but WebDriverSelenium.get_screenshot can raise other exceptions such as
TimeoutException, WebDriverException, and StaleElementReferenceException. This
leads to unhandled exceptions that cause the Celery task to fail without
logging errors, breaking proper error handling for cache warmup failures. The
fix changes the exception to catch broader exceptions to ensure all failures
are logged and tracked.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
except Exception:
logger.exception("Error warming up cache!")
````
</div>
</details>
</div>
<small><i>Code Review Run <a
href=https://github.com/apache/superset/pull/35946#issuecomment-3478922652>#bf792c</a></i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
