sunank200 commented on code in PR #58227:
URL: https://github.com/apache/airflow/pull/58227#discussion_r2542902675
##########
task-sdk/src/airflow/sdk/execution_time/task_runner.py:
##########
@@ -1407,9 +1407,17 @@ def finalize(
task = ti.task
# Pushing xcom for each operator extra links defined on the operator only.
for oe in task.operator_extra_links:
- link, xcom_key = oe.get_link(operator=task, ti_key=ti), oe.xcom_key #
type: ignore[arg-type]
- log.debug("Setting xcom for operator extra link", link=link,
xcom_key=xcom_key)
- _xcom_push_to_db(ti, key=xcom_key, value=link)
+ try:
+ link, xcom_key = oe.get_link(operator=task, ti_key=ti),
oe.xcom_key # type: ignore[arg-type]
+ log.debug("Setting xcom for operator extra link", link=link,
xcom_key=xcom_key)
+ _xcom_push_to_db(ti, key=xcom_key, value=link)
+ except Exception:
+ log.exception(
+ "Failed to get link for operator extra link",
Review Comment:
changed it
##########
task-sdk/tests/task_sdk/execution_time/test_task_runner.py:
##########
@@ -1818,6 +1819,90 @@ def execute(self, context):
map_index=runtime_ti.map_index,
)
+ def test_task_failed_with_operator_extra_links(
+ self, create_runtime_ti, mock_supervisor_comms, time_machine
+ ):
+ """Test that operator extra links are pushed to xcoms even when task
fails."""
+ instant = timezone.datetime(2024, 12, 3, 10, 0)
+ time_machine.move_to(instant, tick=False)
+
+ class DummyTestOperator(BaseOperator):
+ operator_extra_links = (AirflowLink(),)
+
+ def execute(self, context):
+ raise ValueError("Task failed intentionally")
+
+ task = DummyTestOperator(task_id="task_with_operator_extra_links")
+ runtime_ti = create_runtime_ti(task=task)
+ context = runtime_ti.get_template_context()
+ runtime_ti.start_date = instant
+ runtime_ti.end_date = instant
+
+ state, _, error = run(runtime_ti, context=context,
log=mock.MagicMock())
+ assert state == TaskInstanceState.FAILED
+ assert error is not None
+
+ with mock.patch.object(XCom, "_set_xcom_in_db") as mock_xcom_set:
+ finalize(
+ runtime_ti,
+ log=mock.MagicMock(),
+ state=TaskInstanceState.FAILED,
+ context=context,
+ error=error,
+ )
+ mock_xcom_set.assert_called_once_with(
+ key="_link_AirflowLink",
+ value="https://airflow.apache.org",
+ dag_id=runtime_ti.dag_id,
+ task_id=runtime_ti.task_id,
+ run_id=runtime_ti.run_id,
+ map_index=runtime_ti.map_index,
+ )
+
+ def test_operator_extra_links_exception_handling(
+ self, create_runtime_ti, mock_supervisor_comms, time_machine
+ ):
+ """Test that exceptions in get_link() don't prevent other links from
being pushed."""
+ instant = timezone.datetime(2024, 12, 3, 10, 0)
+ time_machine.move_to(instant, tick=False)
+
+ class FailingLink(BaseOperatorLink):
+ """A link that raises an exception when get_link is called."""
+
+ name = "failing_link"
+
+ def get_link(self, operator, *, ti_key):
+ raise ValueError("Link generation failed")
+
+ class DummyTestOperator(BaseOperator):
+ operator_extra_links = (FailingLink(), AirflowLink())
+
+ def execute(self, context):
+ pass
+
+ task = DummyTestOperator(task_id="task_with_multiple_links")
+ runtime_ti = create_runtime_ti(task=task)
+ context = runtime_ti.get_template_context()
+ runtime_ti.start_date = instant
+ runtime_ti.end_date = instant
+
+ with mock.patch.object(XCom, "_set_xcom_in_db") as mock_xcom_set:
+ finalize(
+ runtime_ti,
+ log=mock.MagicMock(),
+ state=TaskInstanceState.SUCCESS,
+ context=context,
+ )
+ assert mock_xcom_set.call_count == 1
+ mock_xcom_set.assert_called_with(
+ key="_link_AirflowLink",
+ value="https://airflow.apache.org",
+ dag_id=runtime_ti.dag_id,
+ task_id=runtime_ti.task_id,
+ run_id=runtime_ti.run_id,
+ map_index=runtime_ti.map_index,
+ )
Review Comment:
changed it
##########
task-sdk/tests/task_sdk/execution_time/test_task_runner.py:
##########
@@ -1818,6 +1819,90 @@ def execute(self, context):
map_index=runtime_ti.map_index,
)
+ def test_task_failed_with_operator_extra_links(
+ self, create_runtime_ti, mock_supervisor_comms, time_machine
+ ):
+ """Test that operator extra links are pushed to xcoms even when task
fails."""
+ instant = timezone.datetime(2024, 12, 3, 10, 0)
+ time_machine.move_to(instant, tick=False)
+
+ class DummyTestOperator(BaseOperator):
+ operator_extra_links = (AirflowLink(),)
+
+ def execute(self, context):
+ raise ValueError("Task failed intentionally")
+
+ task = DummyTestOperator(task_id="task_with_operator_extra_links")
+ runtime_ti = create_runtime_ti(task=task)
+ context = runtime_ti.get_template_context()
+ runtime_ti.start_date = instant
+ runtime_ti.end_date = instant
+
+ state, _, error = run(runtime_ti, context=context,
log=mock.MagicMock())
+ assert state == TaskInstanceState.FAILED
+ assert error is not None
+
+ with mock.patch.object(XCom, "_set_xcom_in_db") as mock_xcom_set:
+ finalize(
+ runtime_ti,
+ log=mock.MagicMock(),
+ state=TaskInstanceState.FAILED,
+ context=context,
+ error=error,
+ )
+ mock_xcom_set.assert_called_once_with(
+ key="_link_AirflowLink",
+ value="https://airflow.apache.org",
+ dag_id=runtime_ti.dag_id,
+ task_id=runtime_ti.task_id,
+ run_id=runtime_ti.run_id,
+ map_index=runtime_ti.map_index,
+ )
Review Comment:
changed it
--
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]