michael-s-molina commented on PR #36872:
URL: https://github.com/apache/superset/pull/36872#issuecomment-3710520004
## 1. Terminal State Handling (Medium Priority)
**Location:** `trino.py:196, 222`
```python
state = "QUEUED"
# ...
while state != "FINISHED":
# ...
state = info.get("state", "UNKNOWN")
```
**Issue:** The loop only exits on `state == "FINISHED"`, but Trino queries
can end in other terminal states like `"FAILED"` or `"CANCELED"`. If the query
fails, the loop relies entirely on `execute_event.is_set()` to break out.
**Recommendation:** Check for all terminal states:
```python
TERMINAL_STATES = {"FINISHED", "FAILED", "CANCELED"}
while state not in TERMINAL_STATES:
```
## 2. No Maximum Timeout Safeguard (Low Priority)
**Location:** `trino.py:199-231`
**Issue:** The polling loop has no maximum iteration count or timeout. While
the `execute_event` safeguard should prevent infinite loops, a defensive
timeout would add resilience.
**Recommendation:** Consider using the existing
`SQLLAB_ASYNC_TIME_LIMIT_SEC` config (default 6 hours) as a fallback. This is
the appropriate config since the Trino progress polling happens during async
execution:
```python
max_wait_time = app.config.get("SQLLAB_ASYNC_TIME_LIMIT_SEC", 21600)
start_time = time.time()
while state != "FINISHED":
if time.time() - start_time > max_wait_time:
logger.warning("Query %d: Progress polling timed out", query.id)
break
# ... rest of loop
```
## 3. Potential Type Error on `totalSplits` (Low Priority)
**Location:** `trino.py:224`
```python
total_splits = float(info.get("totalSplits", 1))
```
**Issue:** If `totalSplits` is explicitly `None` in the stats dict (rather
than missing), `float(None)` will raise a `TypeError`.
**Recommendation:** Use a safer default:
```python
total_splits = float(info.get("totalSplits") or 1)
```
--
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]
