Copilot commented on code in PR #63207:
URL: https://github.com/apache/airflow/pull/63207#discussion_r3066787286
##########
task-sdk/tests/task_sdk/execution_time/test_task_runner.py:
##########
@@ -4448,6 +4448,25 @@ def test_handle_trigger_dag_run_conflict(
]
mock_supervisor_comms.assert_has_calls(expected_calls)
+ @time_machine.travel("2025-01-01 00:00:00", tick=False)
+ def test_handle_trigger_dag_run_dag_not_found(self, create_runtime_ti,
mock_supervisor_comms):
+ """Test that TriggerDagRunOperator fails gracefully when the target
DAG doesn't exist."""
+ from airflow.providers.standard.operators.trigger_dagrun import
TriggerDagRunOperator
+
+ task = TriggerDagRunOperator(
+ task_id="test_task",
+ trigger_dag_id="nonexistent_dag",
+ trigger_run_id="test_run_id",
+ )
+ ti = create_runtime_ti(dag_id="test_handle_trigger_dag_run_not_found",
run_id="test_run", task=task)
+
+ log = mock.MagicMock()
+ mock_supervisor_comms.send.return_value =
ErrorResponse(error=ErrorType.DAG_NOT_FOUND)
+ state, msg, _ = run(ti, ti.get_template_context(), log)
Review Comment:
New test introduces an unspecced `mock.MagicMock()` for `log`. Airflow’s
testing guidelines prefer mocks with `spec`/`autospec` to avoid silently
accepting unexpected attributes; consider using a real
`structlog.get_logger(...)` or `mock.Mock(spec=...)` for the logger instead.
##########
task-sdk/src/airflow/sdk/api/client.py:
##########
@@ -724,6 +724,9 @@ def trigger(
f"dag-runs/{dag_id}/{run_id}",
content=body.model_dump_json(exclude_defaults=True)
)
except ServerResponseError as e:
+ if e.response.status_code == HTTPStatus.NOT_FOUND:
+ log.error("Dag not found.", dag_id=dag_id)
+ return ErrorResponse(error=ErrorType.DAG_NOT_FOUND)
Review Comment:
The new 404 handling logs only a generic "Dag not found" message and doesn’t
include available response context (`e.detail`, `status_code`). Consider
logging those fields (and/or returning them in `ErrorResponse.detail`) for
easier debugging, consistent with other client methods’ 404 handling.
```suggestion
log.error(
"Dag not found.",
dag_id=dag_id,
status_code=e.response.status_code,
detail=e.detail,
)
return ErrorResponse(error=ErrorType.DAG_NOT_FOUND,
detail=e.detail)
```
--
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]
