codeant-ai-for-open-source[bot] commented on code in PR #36909:
URL: https://github.com/apache/superset/pull/36909#discussion_r2663415309
##########
superset/db_engine_specs/trino.py:
##########
@@ -25,6 +25,7 @@
import requests
from flask import copy_current_request_context, ctx, current_app as app,
Flask, g
+from flask_babel import gettext as __
Review Comment:
**Suggestion:** Unguarded import of flask_babel: importing `gettext` from
`flask_babel` without a fallback will raise ImportError when `flask_babel` is
not installed and will also use immediate translation (which can require an
app/request context). Wrap the import in a try/except and prefer `lazy_gettext`
when available, with a safe no-op fallback so the module can be imported in
environments where flask_babel is not present and translations won't require a
request context. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
try:
# Prefer lazy_gettext so translations are evaluated lazily (safe outside
request context)
from flask_babel import lazy_gettext as __
except Exception:
# Fallback to a no-op translator if flask_babel isn't available to avoid
import errors
def __(s):
return s
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This is a reasonable safety and correctness improvement: using lazy_gettext
avoids evaluating translations at import time (which can require an app/request
context) and wrapping the import prevents ImportError in environments where
flask_babel isn't installed. The change addresses a real risk in how
translations are evaluated and makes the module more robust.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/trino.py
**Line:** 28:28
**Comment:**
*Possible Bug: Unguarded import of flask_babel: importing `gettext`
from `flask_babel` without a fallback will raise ImportError when `flask_babel`
is not installed and will also use immediate translation (which can require an
app/request context). Wrap the import in a try/except and prefer `lazy_gettext`
when available, with a safe no-op fallback so the module can be imported in
environments where flask_babel is not present and translations won't require a
request context.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
##########
superset/db_engine_specs/trino.py:
##########
@@ -225,14 +230,22 @@ def handle_cursor(cls, cursor: Cursor, query: Query) ->
None:
)
break
+ needs_commit = False
info = getattr(cursor, "stats", {}) or {}
state = info.get("state", "UNKNOWN")
completed_splits = float(info.get("completedSplits", 0))
total_splits = float(info.get("totalSplits", 1) or 1)
progress = math.floor((completed_splits / (total_splits or 1)) *
100)
Review Comment:
**Suggestion:** Unsafe conversion to float:
`float(info.get("completedSplits", 0))` will raise a TypeError if
`info["completedSplits"]` exists but is `None` (float(None) is invalid), and
the progress calculation may divide by zero if `totalSplits` is 0. Coerce None
to numeric defaults using `or` and guard the division to avoid
ZeroDivisionError. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
completed_splits = float(info.get("completedSplits") or 0)
total_splits = float(info.get("totalSplits") or 1)
if total_splits == 0:
progress = 0
else:
progress = math.floor((completed_splits / total_splits) *
100)
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The existing code can raise TypeError if info['completedSplits'] exists but
is None (float(None)). The suggested change (coerce None to 0 and guard
division) prevents TypeError and explicitly handles total_splits == 0 to avoid
ZeroDivisionError, improving robustness of progress calculation.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/trino.py
**Line:** 236:238
**Comment:**
*Possible Bug: Unsafe conversion to float:
`float(info.get("completedSplits", 0))` will raise a TypeError if
`info["completedSplits"]` exists but is `None` (float(None) is invalid), and
the progress calculation may divide by zero if `totalSplits` is 0. Coerce None
to numeric defaults using `or` and guard the division to avoid
ZeroDivisionError.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
