Skip to content

Handle executor events in dag.test()#41625

Merged
vincbeck merged 3 commits intoapache:mainfrom
aws-mwaa:vincbeck/dag_test_executor_events
Aug 22, 2024
Merged

Handle executor events in dag.test()#41625
vincbeck merged 3 commits intoapache:mainfrom
aws-mwaa:vincbeck/dag_test_executor_events

Conversation

@vincbeck
Copy link
Contributor

@vincbeck vincbeck commented Aug 20, 2024

If the task did not reach the task runner environment and fails, no update is made in the DB. The executor is sending "events" that are handled by the scheduler. In dag.test(), there is no scheduler, therefore we need to handle them there as well.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added the area:Scheduler including HA (high availability) scheduler label Aug 20, 2024
@vincbeck vincbeck requested a review from o-nikolas August 20, 2024 20:05
Copy link
Contributor

@o-nikolas o-nikolas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any way to add some tests so that this doesn't regress again? Does the dag.test code itself even have unit tests?

@vincbeck
Copy link
Contributor Author

Any way to add some tests so that this doesn't regress again? Does the dag.test code itself even have unit tests?

It does but we are not testing dag.test when use_executor is True. The reason why it is not tested is it is quite hard to run an executor in pytest. Even LocalExecutor fails. And mocking the executor would result in not testing the feature. I think this should be belong to system test where an actual executor is run.

@vincbeck vincbeck requested a review from dstandish August 21, 2024 21:37
@vincbeck vincbeck merged commit aa9da93 into apache:main Aug 22, 2024
@vincbeck vincbeck deleted the vincbeck/dag_test_executor_events branch August 22, 2024 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:Scheduler including HA (high availability) scheduler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants