kaxil commented on code in PR #64054:
URL: https://github.com/apache/airflow/pull/64054#discussion_r3126577719
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/xcoms.py:
##########
@@ -182,29 +201,29 @@ def get_mapped_xcom_by_slice(
query = query.offset(-stop)
elif start >= 0:
query = query.order_by(XComModel.map_index.asc())
- if (stop := params.stop) is None:
+ if stop is None:
if step >= 0:
query = query.offset(start)
else:
query = query.limit(start + 1)
else:
if stop < 0:
- stop += get_query_count(query, session=session)
+ stop = total_count + stop if total_count is not None else stop
Review Comment:
The `else stop` branch here is unreachable. At this point we're inside `elif
start >= 0:` (so `start` is set), inside the `else:` at line 209 (so `stop` is
set), and inside `if stop < 0:` -- which is exactly the `bool(start >= 0) !=
bool(stop >= 0)` condition at line 179 that sets `total_count`. So `total_count
is not None` is always true here.
The defensive fallback just makes the read harder. You can drop it:
```python
stop += total_count
```
##########
airflow-core/src/airflow/api_fastapi/execution_api/routes/xcoms.py:
##########
@@ -182,29 +201,29 @@ def get_mapped_xcom_by_slice(
query = query.offset(-stop)
elif start >= 0:
query = query.order_by(XComModel.map_index.asc())
- if (stop := params.stop) is None:
+ if stop is None:
if step >= 0:
query = query.offset(start)
else:
query = query.limit(start + 1)
else:
if stop < 0:
- stop += get_query_count(query, session=session)
+ stop = total_count + stop if total_count is not None else stop
if step >= 0:
query = query.slice(start, stop)
else:
query = query.slice(stop + 1, start + 1)
else:
query = query.order_by(XComModel.map_index.desc())
step = -step
- if (stop := params.stop) is None:
+ if stop is None:
if step > 0:
query = query.offset(-start - 1)
else:
query = query.limit(-start)
else:
if stop >= 0:
- stop -= get_query_count(query, session=session)
+ stop = stop - total_count if total_count is not None else stop
Review Comment:
Same as the symmetric branch at line 211: we're inside `else:` (start < 0),
inside `else:` at line 224 (stop is set), and inside `if stop >= 0:`. That
matches the `bool(start >= 0) != bool(stop >= 0)` guard at line 179, so
`total_count is not None` is always true here and the `else stop` fallback is
dead.
Simpler:
```python
stop -= total_count
```
--
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]
