You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "kesem0811 (via GitHub)" <gi...@apache.org> on 2026/06/20 18:56:10 UTC
[PR] Add unit tests for Kubernetes xcom sidecar helper [airflow]
kesem0811 opened a new pull request, #68786:
URL: https://github.com/apache/airflow/pull/68786
add_xcom_sidecar is used by KubernetesPodOperator when do_xcom_push=True to attach the xcom sidecar and shared emptyDir volume, but it had no dedicated unit tests. This PR adds coverage for that util.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix pod mutation bug in add_xcom_sidecar and add unit tests for Kubernetes xcom sidecar helper [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed pull request #68786: Fix pod mutation bug in add_xcom_sidecar and add unit tests for Kubernetes xcom sidecar helper
URL: https://github.com/apache/airflow/pull/68786
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add unit tests for Kubernetes xcom sidecar helper [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #68786:
URL: https://github.com/apache/airflow/pull/68786#issuecomment-5154210082
The tests are welcome, but the one-line change in this PR is doing more than the title and description suggest, and I think it deserves to be the headline.
```python
pod_cp = copy.deepcopy(pod)
pod_cp.spec.volumes = pod.spec.volumes or [] # reads the original, not the copy
pod_cp.spec.volumes.insert(0, PodDefaults.VOLUME)
```
Rebinding to the *original's* list defeats the deepcopy for that attribute, so the `insert` mutates the caller's pod:
```
before: caller pod after add_xcom_sidecar() -> ['XCOM_VOLUME', 'user-volume']
after: caller pod after add_xcom_sidecar() -> ['user-volume']
```
`add_xcom_sidecar` deepcopies specifically so it does not touch its input, and that contract was silently broken for any pod that already had volumes. It stays invisible when there are none, because `or []` hands back a fresh list.
Could you retitle and describe this as the bug fix it is, with the tests as supporting work? The description becomes the squash commit message, and as it stands someone bisecting a pod-mutation problem would never find this commit.
Two practical notes, neither about the content.
This branch has drifted a long way behind `main` — 1047 commits — and is now conflicting, so it needs a rebase before it can go anywhere.
And this and #68788 both edit `xcom_sidecar.py` and `test_xcom_sidecar.py`, so whichever merges first will leave the other needing a second rebase. My suggestion is to land this one first: it is small and independently correct, and #68788's larger `PodDefaults` restructuring then rebases onto it rather than the other way round. I have left a note there too.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix pod mutation bug in add_xcom_sidecar and add unit tests for Kubernetes xcom sidecar helper [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #68786:
URL: https://github.com/apache/airflow/pull/68786#issuecomment-5997044168
Thanks for finding and fixing this! The same bug was fixed in #72522, merged on 2026-09-09: it changes only the deep copy and also copies `PodDefaults.VOLUME`, with tests in the same `test_xcom_sidecar.py`. That's why this branch now conflicts with `main`. Closing as superseded; your fix was spot on.
---
Drafted-by: Claude Code (Opus 5.5); reviewed by @potiuk before posting
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org