manipatnam commented on code in PR #62401:
URL: https://github.com/apache/airflow/pull/62401#discussion_r2876053980
##########
providers/cncf/kubernetes/docs/operators.rst:
##########
@@ -155,6 +155,37 @@ Example to fetch and display container log periodically
:end-before: [END howto_operator_async_log]
+Pod cleanup on kill
+^^^^^^^^^^^^^^^^^^^
+
+The ``cancel_on_kill`` parameter controls whether the Kubernetes pod is
deleted when a
+running task is killed (e.g. manually marked as success or failed from the
Airflow UI).
+
+In **sync mode**, ``cancel_on_kill`` gates the ``on_kill`` callback: when
``True`` (the
+default), killing the task deletes the pod; when ``False``, the pod is left
running.
+
+In **deferrable mode**, ``cancel_on_kill`` is forwarded to the trigger. When
the trigger
+is cancelled it will clean up the pod according to the ``on_finish_action``
setting:
+
+- ``on_finish_action="delete_pod"`` (default): the pod is deleted
+- ``on_finish_action="keep_pod"``: the pod is left running
+- ``on_finish_action="delete_succeeded_pod"``: the pod is left running
Review Comment:
Since I modified the flow I think we no longer need to add this
New flow:
In **deferrable mode**, ``cancel_on_kill`` is forwarded to the trigger. When
the trigger is cancelled (e.g. the deferred task is manually marked as success
or failed), the pod is deleted. The ``on_finish_action`` parameter is **not**
consulted during a kill -- it only governs cleanup after normal task completion.
##########
providers/cncf/kubernetes/docs/operators.rst:
##########
@@ -155,6 +155,37 @@ Example to fetch and display container log periodically
:end-before: [END howto_operator_async_log]
+Pod cleanup on kill
+^^^^^^^^^^^^^^^^^^^
+
+The ``cancel_on_kill`` parameter controls whether the Kubernetes pod is
deleted when a
+running task is killed (e.g. manually marked as success or failed from the
Airflow UI).
+
+In **sync mode**, ``cancel_on_kill`` gates the ``on_kill`` callback: when
``True`` (the
+default), killing the task deletes the pod; when ``False``, the pod is left
running.
+
+In **deferrable mode**, ``cancel_on_kill`` is forwarded to the trigger. When
the trigger
+is cancelled it will clean up the pod according to the ``on_finish_action``
setting:
Review Comment:
There's no constraint that requires different behaviour between modes.
You're right that it should be symmetric.
I've updated the implementation so both modes are now consistent:
`cancel_on_kill` is the sole control for kill behaviour, and `on_finish_action`
is not consulted during a kill in either mode. It only governs cleanup after
normal task completion.
So the behaviour is now:
Kill path (sync `on_kill()` / deferrable `cleanup()`): controlled
exclusively by `cancel_on_kill`. When True, the pod is always deleted
regardless of `on_finish_action`. When False, the pod is left running.
Normal completion path: controlled by `on_finish_action` as before
(delete_pod, keep_pod, delete_succeeded_pod, delete_active_pod).
This gives a clean separation -- `cancel_on_kill` for kills,
`on_finish_action` for normal completion.
--
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]
