o-nikolas commented on code in PR #28827:
URL: https://github.com/apache/airflow/pull/28827#discussion_r1066407359
##########
airflow/providers/amazon/aws/utils/waiter.py:
##########
@@ -60,14 +60,20 @@ def waiter(
break
if state in failure_states:
raise AirflowException(f"{object_type.title()} reached failure
state {state}.")
- if countdown > check_interval_seconds:
- countdown -= check_interval_seconds
+
+ if countdown is None:
Review Comment:
> Small suggestion though, I think defaulting the waiter countdown to
infinity would be a better default. 25 minutes is such an arbitrary number,
plus there's already task.execution_timeout and I don't see the need for yet
another limit.
Yeah, fundamentally I agree with you! But also it's a tiny bit of backcompat
at this point. There could be folks who are depending on this timeout, and
their workflows may start to fail (or start to unexpectedly pass haha) if that
default switches to infinite. Also all boto waiters get the default timeouts
from the service teams, and I don't usually see them as being infinite (though
there are lots I haven't seen of course!), so it'd also be a little bit of an
unusual behaviour for a waiter.
But these are small concerns, if you feel strongly about the new default,
I'm happy to commit to it :+1:
> So instead of requiring a user to configure countdown=float("inf")
In either approach I think we should allow None and then cast that to inf
inside the waiter for convenience.
--
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]
