potiuk commented on PR #68788:
URL: https://github.com/apache/airflow/pull/68788#issuecomment-5154210142

   Fixing #39184 is worthwhile — tasks hanging indefinitely because no sidecar 
is injected to read `/airflow/xcom/return.json` is a bad failure mode, since 
there is nothing in the logs pointing at the cause.
   
   Two practical notes before I review the substance.
   
   This branch is 1047 commits behind `main` and currently conflicting, so it 
needs a rebase.
   
   More importantly, it overlaps with #68786 from you: both edit 
`xcom_sidecar.py` and `tests/.../test_xcom_sidecar.py`. That PR turns out to 
contain a genuine caller-mutation fix — `add_xcom_sidecar` was rebinding 
`pod_cp.spec.volumes` to the *original* pod's list, so the `insert` mutated the 
caller's pod despite the deepcopy — and it is a one-line change with tests 
around it.
   
   My suggestion is to land #68786 first and rebase this one on top. It is the 
smaller and independently correct change, and this PR's `PodDefaults` 
restructuring (`VOLUME_MOUNT_NAME`, `XCOM_SIDECAR_COMMAND`, the `ApiClient` 
import) is much easier to rebase onto that one line than the reverse. Doing it 
the other way round means resolving the same test file twice.
   
   Once the ordering is settled and this is rebased, I will review the 
SparkKubernetesOperator side properly.
   
   ---
   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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to