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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
