kaxil commented on code in PR #70096:
URL: https://github.com/apache/airflow/pull/70096#discussion_r3624822602
##########
providers/common/ai/src/airflow/providers/common/ai/toolsets/sql.py:
##########
@@ -257,7 +254,7 @@ async def get_tools(self, ctx: RunContext[Any]) ->
dict[str, ToolsetTool[Any]]:
toolset=self,
tool_def=tool_def,
max_retries=1,
- args_validator=_PASSTHROUGH_VALIDATOR,
+ args_validator=build_args_validator(schema),
Review Comment:
On the LangChain bridge (`airflow_toolset_to_langchain_tools`, not in this
diff), the new strict validator regresses malformed SQL calls from a bounded
retry to an uncaught crash.
`_build_structured_tool._validate` runs
`args_validator.validate_python(kwargs)` on the raw args LangChain hands in,
and LangChain does not itself enforce the dict `args_schema`, so a call that
omits `sql` now raises `pydantic_core.ValidationError` right there. But
`_sync_call`/`_async_call` (langchain_bridge.py:177, 185) only catch
`ModelRetry`, so it propagates uncaught and aborts the run.
Before this PR the passthrough validator let those args reach `call_tool`,
where the `KeyError` was caught and turned into a bounded `ModelRetry`
(sql.py:278-288). SQL is the one clean regression here: `HookToolset`
(`method(**tool_args)` -> `TypeError`) and `DataFusionToolset`
(`tool_args["sql"]` outside the inner `try`) already crashed uncaught on
missing args, so their bridge behaviour is unchanged.
pydantic-ai's own `ToolManager` catches `ValidationError` and turns it into
a bounded retry, which is exactly the behaviour this PR enables on the native
path. Catching `ValidationError` alongside `ModelRetry` in
`_sync_call`/`_async_call` and routing it through `_handle_retry` would restore
that on the bridge too. Non-blocking: the native pydantic-ai path this PR
targets is correct and well tested.
--
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]
