Copilot commented on code in PR #65741:
URL: https://github.com/apache/airflow/pull/65741#discussion_r3140142798
##########
providers/cncf/kubernetes/src/airflow/providers/cncf/kubernetes/triggers/pod.py:
##########
@@ -411,17 +414,52 @@ async def get_task_state(self):
async def safe_to_cancel(self) -> bool:
"""
- Whether it is safe to cancel the external job which is being executed
by this trigger.
+ Whether it is safe to delete the pod during trigger cleanup.
- Cancel is NOT safe when the task is still in DEFERRED state, because
it means the
- triggerer is redistributing triggers and the trigger will be recreated
on another triggerer.
- Cancel IS safe when the task state has changed (e.g. user marked it as
success/failed).
+ Used only on Airflow < 3.3.0 where the triggerer does not invoke
``on_kill()`` for user kills.
+ Cancel is NOT safe when the task is still in DEFERRED state (triggerer
restart).
Review Comment:
In `safe_to_cancel()` docstring the wording still refers to "Cancel" even
though the method is now described as determining whether it's safe to *delete
the pod* during cleanup. Please update the docstring language (e.g. "Deletion
is NOT safe...") to match the updated semantics and avoid confusion with
`on_kill()`.
```suggestion
Deletion is NOT safe when the task is still in DEFERRED state
(triggerer restart).
```
--
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]
