shahar1 commented on code in PR #31258:
URL: https://github.com/apache/airflow/pull/31258#discussion_r1192782693
##########
airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -883,6 +878,22 @@ def build_pod_request_obj(self, context: Context | None =
None) -> k8s.V1Pod:
pod_mutation_hook(pod)
return pod
+ def _add_xcom_sidecar(self, pod):
+ """Add xcom sidecar to pod."""
+ sidecar_container_image = None
+ sidecar_container_resources = None
+ # self.hook may not be subclass of KubernetesHook (see
GKEStartPodOperator) so we must
+ # check to make sure these methods exist before calling
+ if hasattr(self.hook, "get_xcom_sidecar_container_image"):
+ sidecar_container_image =
self.hook.get_xcom_sidecar_container_image()
+ if hasattr(self.hook, "get_xcom_sidecar_container_resources"):
+ sidecar_container_resources =
self.hook.get_xcom_sidecar_container_resources()
Review Comment:
1. I think that it would be better to replace these two `if` statements with
one `if isinstance(self.hook, GKEPodHook)`. For the long term, maybe we should
consider refactoring the abstraction.
2. Is it possible to unit test it? If so, it would be nice to implement.
--
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]
