Copilot commented on code in PR #54119:
URL: https://github.com/apache/airflow/pull/54119#discussion_r3066480835
##########
airflow-core/src/airflow/api/common/trigger_dag.py:
##########
@@ -87,11 +86,12 @@ def _trigger_dag(
)
coerced_logical_date = timezone.coerce_datetime(logical_date)
data_interval: DataInterval | None =
dag.timetable.infer_manual_data_interval(
- run_after=timezone.coerce_datetime(run_after)
+ run_after=coerced_logical_date
)
else:
data_interval = None
+ run_after = run_after or timezone.coerce_datetime(timezone.utcnow())
run_id = run_id or DagRun.generate_run_id(
Review Comment:
This changes the semantics of `data_interval` inference when `logical_date`
is provided (now based on `logical_date` instead of `run_after`). There don’t
appear to be unit tests covering `_trigger_dag()`/`trigger_dag()` producing the
expected `data_interval_start/end` for a provided `logical_date`. Adding a
focused unit test under `airflow-core/tests/unit/api/common/` would help
prevent regressions (e.g., assert `data_interval_end == logical_date` for a
daily timetable).
##########
airflow-core/src/airflow/api/common/trigger_dag.py:
##########
@@ -87,11 +86,12 @@ def _trigger_dag(
)
coerced_logical_date = timezone.coerce_datetime(logical_date)
data_interval: DataInterval | None =
dag.timetable.infer_manual_data_interval(
- run_after=timezone.coerce_datetime(run_after)
+ run_after=coerced_logical_date
)
Review Comment:
`coerced_logical_date` is annotated as `datetime | None`, but
`infer_manual_data_interval()` requires a non-optional `run_after`. Passing
`coerced_logical_date` directly will fail static type checks (mypy) even though
it’s set in this branch. Consider using a non-optional local variable (e.g.,
assign `coerced_ld = timezone.coerce_datetime(logical_date)` and pass that) or
otherwise narrow/cast before calling the timetable.
--
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]
