kaxil commented on code in PR #70177:
URL: https://github.com/apache/airflow/pull/70177#discussion_r3624729052


##########
providers/anthropic/src/airflow/providers/anthropic/triggers/batch.py:
##########
@@ -29,6 +31,11 @@
 from airflow.triggers.base import BaseTrigger, TriggerEvent
 
 
+def _is_permanent_poll_error(error: Exception) -> bool:
+    """Return whether an Anthropic batch polling error cannot succeed on a 
retry."""
+    return isinstance(error, APIStatusError) and 400 <= error.status_code < 
500 and error.status_code != 429

Review Comment:
   The Anthropic SDK treats 408 (request timeout) and 409 (lock timeout) as 
retryable in its own `_should_retry` (`_base_client.py`), alongside 429 and 
5xx. This helper marks them permanent, so a transient 408/409 that outlasts the 
SDK's internal `max_retries` now fails the whole batch immediately instead of 
being tolerated by the poll loop, which is the opposite of what this PR is 
trying to preserve. Can you exclude 408 and 409 as well? The SDK also honors an 
`x-should-retry` response header (a 400 can be flagged retryable, a 429 
non-retryable), so reusing the SDK's own retry decision would be sturdier than 
hand-rolling the status ranges, but at minimum adding those two codes covers 
the common case.



-- 
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]

Reply via email to