jedcunningham commented on code in PR #30718:
URL: https://github.com/apache/airflow/pull/30718#discussion_r1174614939
##########
airflow/providers/cncf/kubernetes/operators/pod.py:
##########
@@ -373,6 +378,19 @@ def __init__(
self.deferrable = deferrable
self.poll_interval = poll_interval
self.remote_pod: k8s.V1Pod | None = None
+ if is_delete_operator_pod is not None:
+ warnings.warn(
+ "`is_delete_operator_pod` parameter is deprecated, please use
`on_finish_action`",
+ DeprecationWarning,
Review Comment:
```suggestion
DeprecationWarning,
stacklevel=2,
```
We probably should do this (think this is the right level).
##########
tests/providers/cncf/kubernetes/triggers/test_pod.py:
##########
@@ -239,6 +257,52 @@ async def
test_logging_in_trigger_when_cancelled_should_execute_successfully(
assert "Outputting container logs..." in caplog.text
assert "Deleting pod..." in caplog.text
+ @pytest.mark.asyncio
+ @mock.patch(f"{TRIGGER_PATH}._get_async_hook")
+ async def
test_logging_in_trigger_when_cancelled_should_execute_successfully_without_delete_pod(
+ self,
+ mock_hook,
+ caplog,
+ ):
+ """
+ Test that KubernetesPodTrigger fires the correct event in case if the
task was cancelled.
Review Comment:
```suggestion
Test that KubernetesPodTrigger fires the correct event if the task
was cancelled.
```
nit
##########
tests/providers/cncf/kubernetes/operators/test_pod.py:
##########
@@ -1009,16 +1025,50 @@ def test_wait_for_xcom_sidecar_iff_push_xcom(self,
mock_await, mock_extract_xcom
else:
mock_await.assert_not_called()
- @pytest.mark.parametrize("should_fail", [True, False])
+ @pytest.mark.parametrize(
+ "task_kwargs, should_fail, should_be_deleted",
+ [
+ ({}, False, True),
+ ({}, True, True),
+ (
+ {"is_delete_operator_pod": True, "on_finish_action":
"keep_pod"},
+ False,
+ True,
+ ), # check b/c of is_delete_operator_pod
Review Comment:
```suggestion
), # check backcompat of is_delete_operator_pod
```
This might be better, took me a bit to figure out what "b/c" was.
--
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]
