-
Notifications
You must be signed in to change notification settings - Fork 14.8k
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
AWS Batch Executor #37618
AWS Batch Executor #37618
Conversation
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.
Minor things that I've found
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.
I'm halfway through, but need to go for lunch, so I'm "committing" the first batch (no pun intended) of comments :)
Looking great so far!
docs/apache-airflow-providers-amazon/executors/batch-executor.rst
Outdated
Show resolved
Hide resolved
docs/apache-airflow-providers-amazon/executors/batch-executor.rst
Outdated
Show resolved
Hide resolved
tests/providers/amazon/aws/executors/batch/test_batch_executor.py
Outdated
Show resolved
Hide resolved
…eExecutor and BatchExecutor launches Airflow tasks on AWS ECS/Fargate service and the AWS Batch service, respectively.
Rebase from main
Fix import statements order
-Apply recursive updates to exec_config parameter -Apply waits between each task retry attempt -Unit tests
Add doc string to functions
22e5407
to
a2e5d0b
Compare
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.
Looking good to me, if it take in account that executor has an experimental status
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.
Some minor comments on old threads.
Can you add one last change to the docs to list this as an experimental executor (as we currently do for ECS here).
Remove assert statement
Sorry I missed those! Added them now, thanks! |
Overview
This PR introduces the AWS Batch Executor. This Executor can be configured to run Airflow tasks using AWS Batch. It is based on an initial contribution from @aelzeiny.
From the README:
This PR comes with most features now included in the ECS Executor, but we will continue to update both executors as we work to improve them.
Review Notes
Similar to the ECS Executor, this PR comes as a fully functional feature, which is great for testing, but it does mean that the PR is large. When reviewing, it isn't necessary to go through every single line. Instead, it would be more beneficial to have read through the documentation, and glance at how the Executor is implemented.
There are a lot of similarities in the code between the Batch Executor and the ECS Executor - this is by design. As we continue to refine the process of writing custom Executors, we are converging on an optimal framework to write efficient, reliable, and fault-tolerant custom Executors.
Testing
There is extensive unit testing which has near 100% line coverage in most cases:

^ 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.