pierrejeambrun commented on code in PR #46390:
URL: https://github.com/apache/airflow/pull/46390#discussion_r1943187402
##########
airflow/api_fastapi/core_api/routes/public/dag_run.py:
##########
@@ -362,16 +362,19 @@ def trigger_dag_run(
end=pendulum.instance(body.data_interval_end),
)
else:
- data_interval =
dag.timetable.infer_manual_data_interval(run_after=logical_date)
+ data_interval = dag.timetable.infer_manual_data_interval(
+ run_after=logical_date or pendulum.now("UTC")
Review Comment:
I think we should have only one `pendulum.now("UTC")` so it's easy to
identify that all of those were automatically filled with `now`. Here we will
have multiple different `now` values.
##########
airflow/api_fastapi/core_api/routes/public/dag_run.py:
##########
@@ -362,16 +362,19 @@ def trigger_dag_run(
end=pendulum.instance(body.data_interval_end),
)
else:
- data_interval =
dag.timetable.infer_manual_data_interval(run_after=logical_date)
+ data_interval = dag.timetable.infer_manual_data_interval(
+ run_after=logical_date or pendulum.now("UTC")
+ )
- if body.dag_run_id:
- run_id = body.dag_run_id
- else:
- run_id = dag.timetable.generate_run_id(
+ run_id = (
+ body.dag_run_id
+ if body.dag_run_id
+ else dag.timetable.generate_run_id(
Review Comment:
I don't think `body.dag_run_id` can ever be none. It's automatically
validated and provided a value if `if not self.dag_run_id:` via
`DagRun.generate_run_id(`
##########
airflow/models/backfill.py:
##########
@@ -141,6 +140,7 @@ class BackfillDagRunExceptionReason(str, Enum):
IN_FLIGHT = "in flight"
ALREADY_EXISTS = "already exists"
UNKNOWN = "unknown"
+ CLEARED_RUN = "cleared existing run"
Review Comment:
Not sure I understand this addition as part of the PR.
##########
airflow/migrations/versions/0032_3_0_0_rename_execution_date_to_logical_date_and_nullable.py:
##########
@@ -17,9 +17,9 @@
# under the License.
"""
-Drop ``execution_date`` unique constraint on DagRun.
+Make logical_date nullable.
Review Comment:
Editing a migration will not re-run anything for people using breeze. (They
might run into silent errors until they clean their db and re-run migration
from scratch).
That's not a big deal but we might want to drop a message in the contributor
channel.
##########
airflow/models/backfill.py:
##########
@@ -262,7 +266,23 @@ def _create_backfill_dag_run(
dag_run_conf,
backfill_sort_ordinal,
session,
+ from_date,
+ to_date,
):
+ from airflow.models import DAG
Review Comment:
I assume non local import creates a cyclic import ?
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
