o-nikolas commented on code in PR #61145:
URL: https://github.com/apache/airflow/pull/61145#discussion_r2734345293
##########
providers/amazon/src/airflow/providers/amazon/aws/operators/eks.py:
##########
@@ -104,17 +104,43 @@ def _create_compute(
nodeRole=nodegroup_role_arn,
**create_nodegroup_kwargs,
)
- if wait_for_completion:
- log.info("Waiting for nodegroup to provision. This will take some
time.")
- wait(
- waiter=eks_hook.conn.get_waiter("nodegroup_active"),
- waiter_delay=waiter_delay,
- waiter_max_attempts=waiter_max_attempts,
- args={"clusterName": cluster_name, "nodegroupName":
nodegroup_name},
- failure_message="Nodegroup creation failed",
- status_message="Nodegroup status is",
- status_args=["nodegroup.status"],
+ try:
+ if wait_for_completion:
+ log.info("Waiting for nodegroup to provision. This will take
some time.")
+ wait(
+ waiter=eks_hook.conn.get_waiter("nodegroup_active"),
+ waiter_delay=waiter_delay,
+ waiter_max_attempts=waiter_max_attempts,
+ args={"clusterName": cluster_name, "nodegroupName":
nodegroup_name},
+ failure_message="Nodegroup creation failed",
+ status_message="Nodegroup status is",
+ status_args=["nodegroup.status"],
+ )
+ except Exception:
Review Comment:
It seems like we're now issuing a delete request in all cases, without even
trying to determine if the exception was a AWS related error. We're simply
catching any exception and deleting. I could also see users wanting the cadaver
left behind to investigate why exactly the provisioning failed.
Overall I'm not fully +1 on these changes, but curious to hear from
@vincbeck and @ferruzzi
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
