Andrushika commented on code in PR #74755:
URL: https://github.com/apache/airflow/pull/74755#discussion_r4291362077
##########
airflow-core/src/airflow/api_fastapi/execution_api/versions/__init__.py:
##########
@@ -77,6 +78,7 @@
AddArgBindingsToTIRunContext,
AddCallbackRunEndpoint,
AddDagRunNoteUpdateEndpoint,
+ AddExecutionTimeoutToTIRunContext,
Review Comment:
We might need to decide which release this goes into before merging.
2026-10-30 already shipped in 3.4.0b1 and its published supervisor schema has
no `execution_timeout`, while main now adds new changes at 2027-02-28 (#74370).
So either this is backported to v3-4-test before 3.4.0, or it moves to
2027-02-28 here and in the supervisor schema.
##########
task-sdk/tests/task_sdk/execution_time/test_supervisor.py:
##########
@@ -1664,6 +1664,65 @@ def
test_execution_timeout_kills_task_that_does_not_stop(self, mocker):
proc._handle_execution_timeout_if_needed()
mock_kill.assert_called_once()
+ @pytest.mark.parametrize(
+ "server_timeout",
+ [
+ pytest.param(30.0, id="language_sdk_task_with_timeout"),
+ pytest.param(None, id="no_timeout"),
+ ],
+ )
+ def test_execution_timeout_from_server_kills_task_that_does_not_stop(self,
mocker, server_timeout):
Review Comment:
Could we split the `no_timeout` case into its own test? With the `if
server_timeout is None` branch it is hard to see what each case expects.
##########
airflow-core/src/airflow/api_fastapi/execution_api/services/task_instances.py:
##########
@@ -59,3 +59,22 @@ def get_arg_bindings(dag_bag: DBDagBag, ti: Any, *, session:
Session) -> list |
if (task := dag.task_dict.get(ti.task_id)) is None or not task.is_stub:
return None
return task.arg_bindings
+
+
+def get_language_sdk_execution_timeout(dag_bag: DBDagBag, ti: Any, *, session:
Session) -> float | None:
Review Comment:
Nit: the first lines here are the same lookup as `get_arg_bindings`. Maybe a
small helper that returns the stub task, so both can use it?
##########
airflow-core/src/airflow/api_fastapi/execution_api/datamodels/taskinstance.py:
##########
@@ -491,6 +491,14 @@ class TIRunContext(BaseModel):
``None`` for regular tasks and for stub tasks that declare no parameters.
"""
+ execution_timeout: float | None = None
+ """
+ The task's ``execution_timeout`` in seconds, for stub (foreign-runtime)
tasks.
Review Comment:
Nit: this docstring and the two `AddExecutionTimeout...` descriptions still
say stub tasks only. Maybe mention native Dag tasks too, like the helper
docstring now does.
--
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]
