potiuk commented on code in PR #73870:
URL: https://github.com/apache/airflow/pull/73870#discussion_r4184646098
##########
providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py:
##########
@@ -216,6 +234,10 @@ async def async_wait(
break
attempt += 1
else:
+ if all_attempts_no_credentials:
Review Comment:
Same zero-attempts case as in `wait` above.
```suggestion
if all_attempts_no_credentials and last_no_credentials_error is not
None:
```
##########
providers/amazon/src/airflow/providers/amazon/aws/utils/waiter_with_logging.py:
##########
@@ -134,6 +143,10 @@ def wait(
break
attempt += 1
else:
+ if all_attempts_no_credentials:
Review Comment:
When `waiter_max_attempts <= 0`, the `while` loop never runs, so
`all_attempts_no_credentials` is still at its initial `True` here, and the
waiter raises `WaiterNoCredentialsError("... missing credentials: None")`.
Before this PR it raised `WaiterMaxAttemptsError("Waiter error: max attempts
reached")`. That points users at a credentials problem that doesn't exist.
Could you also require that a credentials error was actually seen?
```suggestion
if all_attempts_no_credentials and last_no_credentials_error is not
None:
```
A test with `waiter_max_attempts=0` expecting `WaiterMaxAttemptsError` would
pin it down.
--
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]
