dstandish commented on code in PR #43853:
URL: https://github.com/apache/airflow/pull/43853#discussion_r1839323233
##########
providers/src/airflow/providers/cncf/kubernetes/utils/pod_manager.py:
##########
@@ -692,6 +737,12 @@ def read_pod_logs(
post_termination_timeout=post_termination_timeout,
)
+ @tenacity.retry(stop=tenacity.stop_after_attempt(3),
wait=tenacity.wait_exponential(), reraise=True)
+ def get_init_container_names(self, pod: V1Pod) -> list[str]:
+ """Return container names from the POD except for the
airflow-xcom-sidecar container."""
+ pod_info = self.read_pod(pod)
Review Comment:
Listen, I welcome your contribution. That's how new life and new committers
come into the airflow community. But I don't think it's asking too much to ask
you to think through whether the code you propose to add is actually needed.
If you can't be bothered to think it through, maybe this is not the project for
you.
Indeed it looks like it is not necessary. Look at line 623 of pod.py. We
read the pod there. So not sure why you need to read the pod again to get the
init container names, unless you think the init containers can change between
these two places.
You can't assume that just because the code is in airflow, that means it's
good, or that it was thought through. When we touch code, that's an
opportunity to examine it critically. And when we add new code, that
especially is a time to do so. Otherwise complexity just increases.
--
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]
