dstandish commented on PR #23450: URL: https://github.com/apache/airflow/pull/23450#issuecomment-1159246205
> Hi @dstandish and @jedcunningham. Thank you for your review, and sorry for the delay, it's been a busy week. Going back to this PR, I have made a diagram to clarify it because I think that we can delete this function method call https://github.com/apache/airflow/blob/main/airflow/providers/cncf/kubernetes/operators/kubernetes_pod.py#L418. That point can only be reached when the base container has finished, so we don't need to wait until the pod has finished. This will continue working with single containers, and it will fix the behavior when running with sidecars. So, when there is an xcom sidecar, it is terminated when we call `self.extract_xcom(pod=self.pod)`. So after we terminate xcom sidecar, we need to wait for the pod to complete before proceeding on to `cleanup`. Separately it seems either your description needs updating or the PR because it's talking about adding an "await all containers" option but not seeing that in the PR. It might be helpful, in evaluating your approach, to have an example too, demonstrating how your change would help with multiple containers / sidecars. -- 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]
