Lee-W commented on code in PR #46295:
URL: https://github.com/apache/airflow/pull/46295#discussion_r1940824946
##########
tests/api_fastapi/core_api/routes/public/test_dag_run.py:
##########
@@ -1333,28 +1332,8 @@ def
test_should_response_200_for_duplicate_logical_date(self, test_client):
json={"dag_run_id": RUN_ID_2, "note": note},
)
- assert response_1.status_code == response_2.status_code == 200
- body1 = response_1.json()
- body2 = response_2.json()
-
- for each_run_id, each_body in [(RUN_ID_1, body1), (RUN_ID_2, body2)]:
- assert each_body == {
- "dag_run_id": each_run_id,
- "dag_id": DAG1_ID,
- "logical_date": now,
- "queued_at": now,
- "start_date": None,
- "end_date": None,
- "data_interval_start": now,
- "data_interval_end": now,
- "last_scheduling_decision": None,
- "run_type": "manual",
- "state": "queued",
- "external_trigger": True,
- "triggered_by": "rest_api",
- "conf": {},
- "note": note,
- }
+ assert response_1.status_code == 200
+ assert response_2.status_code == 409
Review Comment:
should we keep the body check?
##########
airflow/models/backfill.py:
##########
@@ -262,7 +266,23 @@ def _create_backfill_dag_run(
dag_run_conf,
backfill_sort_ordinal,
session,
+ from_date,
+ to_date,
Review Comment:
should we add type annotation here?
##########
tests/api_fastapi/core_api/routes/public/test_task_instances.py:
##########
@@ -1027,20 +1028,26 @@ def test_should_respond_200_for_dag_id_filter(self,
test_client, session):
assert count == len(response.json()["task_instances"])
@pytest.mark.parametrize(
- "order_by_field", ["start_date", "logical_date",
"data_interval_start", "data_interval_end"]
+ "order_by_field, date",
Review Comment:
```suggestion
"order_by_field, base_date",
```
base_date might be better. `date` might shadow `from datetime import date`
##########
tests/models/test_dagrun.py:
##########
@@ -355,10 +355,11 @@ def test_dagrun_no_deadlock_with_depends_on_past(self,
dag_maker, session):
run_id="test_dagrun_no_deadlock_1",
start_date=DEFAULT_DATE,
)
- dr2 = dag_maker.create_dagrun_after(
- dr,
+ next_date = DEFAULT_DATE + datetime.timedelta(days=1)
+ dr2 = dag_maker.create_dagrun(
run_id="test_dagrun_no_deadlock_2",
start_date=DEFAULT_DATE + datetime.timedelta(days=1),
+ logical_date=next_date,
Review Comment:
start_date seems to use this next_date as well so we can just reuse it and
would like to know why do we set it to next_date here any date equal or after
start_date?
##########
tests/api_fastapi/core_api/routes/public/test_assets.py:
##########
@@ -149,7 +149,7 @@ def _create_dag_run(session, num: int = 2):
dag_id="source_dag_id",
run_id=f"source_run_id_{i}",
run_type=DagRunType.MANUAL,
- logical_date=DEFAULT_DATE,
+ logical_date=DEFAULT_DATE + timedelta(days=i),
Review Comment:
Why did we change these dates here?
--
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]
