-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
Add wait_for_steps_completion option to EmrCreateJobFlowOperator. #44055
base: main
Are you sure you want to change the base?
Add wait_for_steps_completion option to EmrCreateJobFlowOperator. #44055
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)
|
020522f
to
3329599
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.
Could you cover this new use case in a unit test?
wait_for_completion: bool = False, | ||
wait_for_steps_completion: bool = False, |
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 think both wait_for_completion
and wait_for_steps_completion
can be very confusing.
If we are to support the new functionality I suggest to introduce a new parameter wait_policy
which will accept value of wait_for_steps_completion
, wait_for_steps_completed_and_machine_terminated
similar to what we did with Kubernetes in #30718 when we had more than just a bool option for delete pod.
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 like that idea!
Currently, when using
wait_for_complition=True
it will trigger the node to only wait until the cluster is successfully launched.I added the option
wait_for_steps_completion
so that it will also await the cluster to be terminated successfully.This is useful because it will eliminate the need to have an extra node in your DAG.