[AIRFLOW-XXX] Fix race condition in CI test #4968
Conversation
tests/test_jobs.py
Outdated
There was a problem hiding this comment.
@feng-tao may be necessary to update the docstring of this test case as well? Currently it's DagRuns with one successful and one future root task -> SUCCESS
There was a problem hiding this comment.
I think in real scenario it will only have success state for the dagRun. The issue only happens in unit test as we don't wait / sleep to retrieve the result during CI test for speeding up the CI test effort hence we sometimes(or in some ORM) get the running state for the task which the dagrun state is still running.
There was a problem hiding this comment.
Not sure if I'm missing anything, but I'm thinking if it would be better/clearer to just add a sleep here If the race condition only occurs in this test case?
There was a problem hiding this comment.
@XD-DENG , you could take a look at more on this pr(https://github.com/apache/airflow/pull/4730/files) as it tries to speed up the CI time.
There was a problem hiding this comment.
Very similar to https://github.com/apache/airflow/pull/4737/files
feng-tao
force-pushed
the
tfeng_fix_ci_race
branch
from
5a0dcf7 to
d5e6c47
Compare
Codecov Report
@@ Coverage Diff @@
## master #4968 +/- ##
==========================================
+ Coverage 75.67% 75.76% +0.08%
==========================================
Files 458 458
Lines 29856 29856
==========================================
+ Hits 22594 22620 +26
+ Misses 7262 7236 -26
Continue to review full report at Codecov.
|
|
Awesome @feng-tao |
5 participants
After #4808 is merged, we seem to hit a race condition in one of the test. I think in unit test mode we disable the sleeep in https://github.com/apache/airflow/pull/4730/files, I think it is fine for the task to be in either running or success state.