Re: [PR] Distinguish credential failures from AWS waiter exhaustion [airflow]

["potiuk (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org