SameerMesiah97 commented on PR #60319: URL: https://github.com/apache/airflow/pull/60319#issuecomment-3734652715
Looks good to me overall. This is a tightly scoped, minimal fix with a new test (`test_extract_xcom_sidecar_terminated`) that exercises the new guard when `container_is_running = False`. The existing tests already cover the `container_is_running = True` path indirectly, which seems acceptable here given the simplicity of the guard. I also appreciated that `container_is_running` is explicitly patched to `True` n the existing `def extract_xcom` tests to avoid unintended short-circuiting. The only non-blocking suggestion I have is to mention these test additions/adjustments in the PR description, as that context helps reduce review friction. -- 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]
