-
Notifications
You must be signed in to change notification settings - Fork 16.3k
dag.test(): Flush session() before processing Executor Event Buffer #59314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
|
potiuk
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually we require unit tests, but dag.test is a long and not unit-testable really, and the dag.test on its own is used to run tests, so it's a bit "who tests the tester".
It would likely be great however (as a follow-up) to add a test to one of our e2e tests to make a complete run of dag_test with executor and without. Not a unit test but still should allow us to actually test the "dag.test()".
I will create an issue for that.
f47b107 to
92f3705
Compare
c58b49f to
552f876
Compare
|
Ok. old issue from the past with timeout - looks good. Merging. |
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
…pache#59314) (cherry picked from commit e7a1e57) Co-authored-by: Judit Novak <judit.novak@gmail.com>
closes: #59074
The issue have been demonstrated on separate branch (including helper code that is not to be merged): juditnovak#2
The fix have been demonstrated to respond to the issue (including helper code that is not to be merged): juditnovak#1
Root cause
The
dag.test()function is using a single session throughout its lifetime. Since data is cached on the session, outdated results are retrieved.dag.test()session. Instead a cached record is returned from an earlier update.Testing
task-sdktargetingdag.test(). I don't know if this may be intentional?use_executor=Trueoption