[GitHub] [airflow] ephraimbuddy commented on a change in pull request #18531: Workaround intermittently failing scheduler test

[GitBox <gi...@apache.org>]

ephraimbuddy commented on a change in pull request #18531:
URL: https://github.com/apache/airflow/pull/18531#discussion_r716273133



##########
File path: tests/jobs/test_scheduler_job.py
##########
@@ -2658,9 +2659,14 @@ def test_do_schedule_max_active_runs_dag_timed_out(self, dag_maker):
         assert run1_ti.state == State.SKIPPED
 
         # Run scheduling again to assert run2 has started
-        self.scheduler_job._do_scheduling(session)
-        run2 = session.merge(run2)
-        session.refresh(run2)
+        for i in range(1, 10):
+            self.scheduler_job._do_scheduling(session)
+            run2 = session.merge(run2)
+            session.refresh(run2)
+            if run2.state == State.QUEUED:
+                sleep(0.1)
+                continue
+            break

Review comment:
       For the code starting from 2661, I would suggest this:
   ```python
   # Run scheduling again to assert run2 has started
   self.scheduler_job._start_queued_dagruns(session)
   session.flush()
   run2 = session.merge(run2)
   session.refresh(run2)
   assert run2.state == State.RUNNING
   ```
   Since this is testing max_active_runs and dag_timeout, I think we don't need to schedule the task instances.
   We can also run the _schedule_dag_run to have it put the ti into scheduled:
   ```python
   # Run scheduling again to assert run2 has started
   self.scheduler_job._start_queued_dagruns(session)
   session.flush()
   self.scheduler_job._schedule_dag_run(run2, session)
   run2 = session.merge(run2)
   session.refresh(run2)
   assert run2.state == State.RUNNING
   run2_ti = run2.get_task_instance(task1.task_id, session)
   assert run2_ti.state == State.SCHEDULED
   ```
   
   What I have observed is that using `_do_scheduling` in tests usually doesn't do what we want. I prefer using `_start_queued_dagrun` to start dagruns instead of using `do_scheduling`. Maybe we should use it here too.
   https://github.com/apache/airflow/blob/2643345e4b72064c605e42901a3dc531e6aa2f4e/tests/jobs/test_scheduler_job.py#L2755




-- 
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-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org