pierrejeambrun commented on code in PR #45731:
URL: https://github.com/apache/airflow/pull/45731#discussion_r1922255923


##########
tests/api_fastapi/core_api/routes/public/test_backfills.py:
##########
@@ -267,6 +267,53 @@ def test_no_schedule_dag(self, session, dag_maker, 
test_client):
         assert response.status_code == 409
         assert response.json().get("detail") == f"{dag.dag_id} has no schedule"
 
+    @pytest.mark.parametrize(
+        "repro_act, repro_exp,run_backwards, status_code",
+        [
+            ("none", ReprocessBehavior.NONE, False, 409),
+            ("completed", ReprocessBehavior.COMPLETED, False, 200),
+            ("completed", ReprocessBehavior.COMPLETED, True, 409),
+        ],
+    )
+    def test_create_backfill_with_depends_on_past(
+        self, repro_act, repro_exp, run_backwards, status_code, session, 
dag_maker, test_client
+    ):
+        with dag_maker(session=session, dag_id="TEST_DAG_1", schedule="0 * * * 
*") as dag:
+            EmptyOperator(task_id="mytask", depends_on_past=True)
+        session.query(DagModel).all()
+        session.commit()
+        from_date = pendulum.parse("2024-01-01")
+        from_date_iso = to_iso(from_date)
+        to_date = pendulum.parse("2024-02-01")
+        to_date_iso = to_iso(to_date)
+        max_active_runs = 5
+        data = {
+            "dag_id": dag.dag_id,
+            "from_date": f"{from_date_iso}",
+            "to_date": f"{to_date_iso}",
+            "max_active_runs": max_active_runs,
+            "run_backwards": run_backwards,
+            "dag_run_conf": {"param1": "val1", "param2": True},
+            "dry_run": False,
+            "reprocess_behavior": repro_act,
+        }
+        response = test_client.post(
+            url="/public/backfills",
+            json=data,
+        )
+        assert response.status_code == status_code
+        if response.status_code == 409 and repro_act == "none":
+            assert (
+                response.json().get("detail")
+                == f"{dag.dag_id} has tasks for which depends_on_past=True. 
You must set reprocess behavior to reprocess completed or reprocess failed."
+            )
+
+        if response.status_code == 409 and repro_act == "completed":

Review Comment:
   If response is not success then handle errors
   ```suggestion
           if response.status_code != 200:
   ```
   
   ```
   if run_backward:
   ...
   else:
   ...
   ```
   
   To follow the same logic as the code implementation. First `if reverse`, 
then `if reprocess_behavior ...`. Swapping the assertion order and if 
conditions makes it harder to read and understand if all codepath are taken 
care of, for no real addition.



-- 
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]

Reply via email to