jscheffl commented on code in PR #43853:
URL: https://github.com/apache/airflow/pull/43853#discussion_r1835495274
##########
kubernetes_tests/test_kubernetes_pod_operator.py:
##########
Review Comment:
I assume the test is mis-placed and it is a tech debt that the K8s tests are
out of the provider package tree. Can you move the new test to
providers/tests/cncf/kubernetes/operators/test_pod.py ?
##########
providers/src/airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -157,7 +157,11 @@ class KubernetesPodOperator(BaseOperator):
:param labels: labels to apply to the Pod. (templated)
:param startup_timeout_seconds: timeout in seconds to startup the pod.
:param startup_check_interval_seconds: interval in seconds to check if the
pod has already started
+ :param get_init_containers_logs: get the stdout of the init containers as
logs of the tasks.
:param get_logs: get the stdout of the base container as logs of the tasks.
+ :param init_container_logs: list of init containers whose logs will be
published to stdout
+ Takes a sequence of containers, a single container name or True. If
True,
+ all the containers logs are published. Works in conjunction with
get_init_containers_logs param.
Review Comment:
Is this parameter not partly redundant with the other bool parameter?
I would assume if there are mutliple init containers I can use this sequence
to limit from whoich init containers I want to have logs. But if I specify a
list I don't need another bool.
On the other hand if I just want to have all init container logs, then the
list need to be filled?
##########
providers/src/airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -655,6 +668,31 @@ def execute_sync(self, context: Context):
if self.do_xcom_push:
return result
+ @tenacity.retry(
+ wait=tenacity.wait_exponential(max=15),
+ retry=tenacity.retry_if_exception_type(PodCredentialsExpiredFailure),
+ reraise=True,
+ )
+ def await_init_containers_completion(self, pod: k8s.V1Pod):
+ try:
+ if self.get_init_containers_logs:
Review Comment:
I think you don't need to check the option setting again here if the calling
method checked before calling this function
--
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]
