shepherd44 opened a new pull request, #72821:
URL: https://github.com/apache/airflow/pull/72821
## Why
When the pod reaches a terminal state before the operator defers,
`invoke_defer_method` calls `trigger_reentry` inline instead of deferring:
```python
if context and (
pod_container_state == ContainerState.TERMINATED or pod_container_state
== ContainerState.FAILED
):
self.log.info("Skipping deferral as pod is already in a terminal state")
self.trigger_reentry(...) # return value dropped
else:
self.defer(trigger=trigger, method_name="trigger_reentry",
timeout=defer_timeout)
```
`trigger_reentry` ends with:
```python
if self.do_xcom_push:
return xcom_sidecar_output
```
That value was dropped at three levels — `invoke_defer_method`,
`execute_async`, and the deferrable branch of `execute` all called down without
returning.
In the regular deferral path Airflow takes the return value of the resume
method (`trigger_reentry`) and stores it as `return_value`, so the loss only
happens on this shortcut. `execute_sync` already returns its result, which is
why non-deferrable runs are unaffected.
The failure is silent. The task still succeeds, and `pod_name` /
`pod_namespace` are pushed separately by `execute_async`, so the XCom entries
look populated:
| | XCom keys |
|---|---|
| shortcut path | `pod_name`, `pod_namespace` |
| regular deferral | `pod_name`, `pod_namespace`, `return_value` |
Downstream tasks pulling that XCom get `None`. In our case a Jinja template
doing `{{ ti.xcom_pull(task_ids="fetch")["data_key"] }}` failed with `'None'
has no attribute 'data_key'`, three retries deep, while the upstream task was
reported as `success`. Retrying cannot help — the XCom is already gone.
Task logs make the two paths easy to tell apart:
```
# affected
Reusing existing pod '...' (phase=Running, reason=) since it is not
terminated or evicted.
Skipping deferral as pod is already in a terminal state
Deleting pod: ...
<- no "Pushing xcom"
# working
Pausing task as DEFERRED.
Deleting pod: ...
Pushing xcom
```
Pods that finish within a second or two hit this often, which is what made
it show up intermittently for us across several DAGs rather than as a hard
failure.
## What
Propagate the return value through the three call sites, and widen the two
return annotations (`-> None` → `-> Any`) accordingly.
Two unit tests are added:
-
`test_invoke_defer_method_returns_trigger_reentry_result_when_pod_already_terminal`
— the inline call's result is returned
- `test_execute_returns_deferrable_result` — `execute` hands the deferrable
result back, as the synchronous branch already does
Both fail on `main` with `assert None == {'key': 'value'}` and pass with the
change.
## Notes
- I could not find an existing issue or PR for this. #72502 fixes a
different XCom-loss path (`container_logs` substring check in the synchronous
branch), and #67226 fixed a related case where the deferrable path dropped
`multiple_outputs` handling.
- The same inline-`trigger_reentry` shortcut exists in
`providers/amazon/.../operators/eks.py`. I have not touched it here to keep
this PR focused; it may be worth a separate look.
## Testing
Ran locally against this branch:
```
pytest
providers/cncf/kubernetes/tests/unit/cncf/kubernetes/operators/test_pod.py
```
247 passed with the change, 245 on unmodified `main` (the two new tests
being the difference). The same 2 failures and 10 errors (`TestSuppress`,
`write_logs`) occur with and without the change on my machine, so they are
unrelated to it — they look like local environment issues rather than
regressions.
`ruff check` and `ruff format --check` pass on both touched files.
## Gen-AI disclosure
This PR was prepared with the assistance of a Gen-AI tool (Claude). The root
cause was located by reading provider source and correlating it against Airflow
API data and task logs from a real deployment where the bug occurred; I
reviewed the diff and the tests, ran them locally, and I am able to explain and
stand behind the change.
--
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]
