ColtenOuO commented on code in PR #71853:
URL: https://github.com/apache/airflow/pull/71853#discussion_r3816898734
##########
providers/common/ai/src/airflow/providers/common/ai/toolsets/hook.py:
##########
@@ -136,7 +141,10 @@ async def call_tool(
) -> Any:
method_name = name.removeprefix(self._tool_name_prefix) if
self._tool_name_prefix else name
method: Callable[..., Any] = getattr(self._hook, method_name)
- result = method(**tool_args)
+ try:
+ result = method(**tool_args)
+ except Exception as e:
+ raise ModelRetry(f"The {name} tool failed: {e}") from e
Review Comment:
Thanks for the review.
I agree narrowing the caught exceptions is a reasonable ask in principle,
but for HookToolset specifically it's not really feasible: it wraps arbitrary
hooks (HTTP, S3, or any custom hook), and each one raises errors in its own way
— we can't know ahead of time what exception types to expect, so there's no
fixed set of "known retryable" exceptions to narrow to (unlike
DataFusionToolset, which only deals with SQL-layer errors it fully controls).
Given max_retries defaults to 1 here, the worst case is one wasted LLM call
before the tool exhausts its retry budget and the task fails normally. I think
that's an acceptable tradeoff given the constraint.
--
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]
