VladaZakharova commented on code in PR #71528:
URL: https://github.com/apache/airflow/pull/71528#discussion_r4156665522


##########
providers/google/src/airflow/providers/google/cloud/operators/kubernetes_engine.py:
##########
@@ -799,6 +800,19 @@ def invoke_defer_method(
         on_finish_action = self.on_finish_action
         if type(on_finish_action) is str and self.on_finish_action not in 
[i.value for i in OnFinishAction]:
             on_finish_action = self.on_finish_action.split(".")[-1].lower()  # 
type: ignore[assignment]
+
+        # Anchor on ti.start_date so the deadline survives re-deferrals.
+        trigger_kwargs = dict(self.trigger_kwargs or {})

Review Comment:
   Does the operator calculate the remaining time (execution_timeout - 
(utcnow() - task_instance.start_date)) before yielding self.defer(...)?



##########
providers/google/src/airflow/providers/google/cloud/operators/kubernetes_engine.py:
##########
@@ -799,6 +800,19 @@ def invoke_defer_method(
         on_finish_action = self.on_finish_action

Review Comment:
   If the trigger yields a timeout event or fails the task, does the operator 
or trigger clean up the Kubernetes/GKE pod?



##########
providers/google/src/airflow/providers/google/cloud/operators/kubernetes_engine.py:
##########
@@ -799,6 +800,19 @@ def invoke_defer_method(
         on_finish_action = self.on_finish_action
         if type(on_finish_action) is str and self.on_finish_action not in 
[i.value for i in OnFinishAction]:
             on_finish_action = self.on_finish_action.split(".")[-1].lower()  # 
type: ignore[assignment]
+
+        # Anchor on ti.start_date so the deadline survives re-deferrals.
+        trigger_kwargs = dict(self.trigger_kwargs or {})
+        defer_timeout: datetime.timedelta | None = None
+        if self.execution_timeout is not None and context is not None:

Review Comment:
   Is this handled at the GKEStartPodOperator level when it should instead be 
fixed upstream in cncf-kubernetes's KubernetesPodOperator / GKEPodTrigger / 
KubernetesPodTrigger? If KPO has the same bug, fixing only GKE leads to 
fragmentation.



-- 
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]

Reply via email to