Lee-W commented on code in PR #76752:
URL: https://github.com/apache/airflow/pull/76752#discussion_r4521313141
##########
providers/common/ai/src/airflow/providers/common/ai/tools/__init__.py:
##########
@@ -151,35 +156,50 @@ def airflow_tools(self) -> Sequence[AirflowTool]:
...
-def collect_tools(sources: Iterable[ToolProvider | AirflowTool]) ->
list[AirflowTool]:
+ToolSource: TypeAlias = (
+ ToolProvider | AirflowTool | FilteredToolset[Any] | PrefixedToolset[Any] |
RenamedToolset[Any]
+)
+"""What a framework adapter accepts: a tool, or a toolset that
``.filtered()``, ``.prefixed()`` and ``.renamed()`` may wrap."""
+
+
+def collect_tools(sources: Iterable[ToolSource]) -> list[AirflowTool]:
"""
Flatten toolsets and individual tools, as a framework adapter receives
them, into tools.
+ A toolset wrapped with ``.filtered()``, ``.prefixed()`` or ``.renamed()``
contributes the
+ tools the wrappers offer, under the names they give them. The
``.filtered()`` function runs
+ once, here, with an inert run context, so a filter that reads per-run
state does not
+ re-evaluate it before each model turn.
+
+ :raises TypeError: when a source is another wrapper, such as
``.approval_required()``, or
+ has no ``airflow_tools()`` method.
:raises MCPToolsetNotSupportedError: when a source is an ``MCPToolset``.
Connect the
framework's own MCP client to the server instead.
:raises ValueError: when two tools share a name. Frameworks route a call
by name, so one
of them would silently never run; give the toolsets different
``tool_prefix`` values,
- where they take one.
+ where they take one, or wrap them with ``.prefixed()``. Also when a
``.renamed()`` map
+ names a tool the toolset does not have.
Review Comment:
The last sentence of the `:raises ValueError:` (**Also when a .renamed() map
names a tool the toolset does not have**) describes a wider check than the code
does.
`RenamedToolset.get_tools` only looks up the names it is offered in its map,
so an entry for a tool the toolset does not have is never read.
`ValueError` in `airflow_tools_of` fires only when an offered name cannot be
mapped back to one of the toolset’s tools, which is the **map written
new-to-original backwards** case.
A mistyped original name stays silent and the tool keeps its original name;
a quick probe of `.filtered(...)` followed by `.renamed({"run_sql": "query"})`
with query filtered away shows no run_sql and no error.
Could you reword this to the real trigger (e.g., **an offered name that a
.renamed() map sends to a name the toolset does not have, such as a map written
backwards**), or add a check for the mistyped key plus a test?
##########
providers/common/ai/tests/unit/common/ai/tools/test_tools.py:
##########
@@ -153,6 +165,183 @@ def airflow_tools(self) -> list[AirflowTool]:
with pytest.raises(NotImplementedError, match="^not yet$"):
_collect_tools([_Unfinished()], mcp_hint="Connect Acme's McpClient
instead.")
+ def test_a_mock_toolset_is_asked_for_its_tools(self):
+ """The mock has ``airflow_tools`` only through ``__getattr__``, which
a Protocol check misses on Python 3.12+."""
+ source = MagicMock(spec=["airflow_tools"])
+ source.airflow_tools.return_value = [_named("a")]
+
+ assert [tool.name for tool in collect_tools([source])] == ["a"]
+
+
+def _sql_toolset(name: str = "Ada") -> SQLToolset:
+ ts = SQLToolset("pg_default")
+ ts._hook = _make_mock_db_hook(records=[(1, name)],
last_description=[("id",), ("name",)])
+ return ts
+
+
+def _names(tools: list[AirflowTool]) -> list[str]:
+ return [tool.name for tool in tools]
+
+
+def _not_query(ctx: RunContext[Any], tool: ToolDefinition) -> bool:
+ return tool.name != "query"
+
+
+async def _not_query_async(ctx: RunContext[Any], tool: ToolDefinition) -> bool:
+ return tool.name != "query"
+
+
+class TestCollectToolsThroughWrappers:
+ @pytest.mark.parametrize("keep", [_not_query, _not_query_async],
ids=["sync", "async"])
+ def test_a_filter_drops_the_tools_it_rejects(self, keep):
+ tools = collect_tools([_sql_toolset().filtered(keep)])
+
+ assert _names(tools) == ["list_tables", "get_schema", "check_query"]
+
+ def test_the_filter_runs_once_with_each_tools_definition(self):
+ seen: list[tuple[RunContext[Any], ToolDefinition]] = []
+
+ def keep_all(ctx: RunContext[Any], tool: ToolDefinition) -> bool:
+ seen.append((ctx, tool))
+ return True
+
+ tools = collect_tools([_sql_toolset().filtered(keep_all)])
+ asyncio.run({tool.name: tool for tool in tools}["query"].call({"sql":
"SELECT 1"}))
+
+ assert [tool.name for _, tool in seen] == ["list_tables",
"get_schema", "query", "check_query"]
+ # The toolset's own definitions, not ones rebuilt from the AirflowTool.
+ assert all(tool.sequential for _, tool in seen)
+ assert all(ctx.deps is None for ctx, _ in seen)
+
+ @pytest.mark.parametrize(
+ ("wrap", "offered", "query_tool"),
+ [
+ pytest.param(
+ lambda ts: ts.prefixed("wh"),
+ ["wh_list_tables", "wh_get_schema", "wh_query",
"wh_check_query"],
+ "wh_query",
+ id="prefixed",
+ ),
+ pytest.param(
+ lambda ts: ts.renamed({"run_sql": "query"}),
+ ["list_tables", "get_schema", "run_sql", "check_query"],
+ "run_sql",
+ id="renamed",
+ ),
+ ],
+ )
+ def test_a_renamed_tool_runs_the_inner_tool(self, wrap, offered,
query_tool):
+ tools = {tool.name: tool for tool in
collect_tools([wrap(_sql_toolset())])}
+
+ result = asyncio.run(tools[query_tool].call({"sql": "SELECT id, name
FROM users"}))
+
+ assert list(tools) == offered
+ assert json.loads(result.content)["rows"] == [[1, "Ada"]]
+
+ def test_nested_wrappers_apply_from_the_inside_out(self):
Review Comment:
Only one nesting order is tested (prefixed, filtered, renamed, filtered).
Worth adding test cases for
* a `.renamed()` inside `.prefixed()`
* a tool whose name already carries the prefix
* a filter that rejects every tool (an empty list going into each adapter,
for example an empty MCP server in claude_agent_sdk)
* a rename that collides with an existing tool (pydantic-ai raises
`UserError`, but nothing tests it here)
##########
providers/common/ai/tests/unit/common/ai/tools/test_tools.py:
##########
@@ -153,6 +165,183 @@ def airflow_tools(self) -> list[AirflowTool]:
with pytest.raises(NotImplementedError, match="^not yet$"):
_collect_tools([_Unfinished()], mcp_hint="Connect Acme's McpClient
instead.")
+ def test_a_mock_toolset_is_asked_for_its_tools(self):
Review Comment:
test_a_mock_toolset_is_asked_for_its_tools passes against the previous
implementation as well (the old code calls source.airflow_tools() directly and
a mock with that method satisfies it), so it does not guard the change;
test_a_wrapper_that_supplies_its_own_tools_is_asked_for_them further down looks
to be in the same position. Would it help to say in the docstrings that they
are regression guards for the new getattr lookup, or to fold them into a case
that fails without the change? The docstring’s claim about Python 3.12+
Protocol checks has no evidence in the test itself.
##########
providers/common/ai/src/airflow/providers/common/ai/tools/_from_toolset.py:
##########
@@ -88,12 +93,102 @@ def airflow_tools_from_toolset(toolset:
AbstractToolset[Any], *, deps: Any = Non
:param toolset: The pydantic-ai toolset to expose.
:param deps: Exposed to the toolset as ``ctx.deps``.
"""
- ctx: RunContext[Any] = RunContext(deps=deps, model=TestModel(),
usage=RunUsage(), max_retries=1)
+ ctx = _inert_context(deps)
toolset_tools = run_coroutine_sync(toolset.get_tools(ctx))
in_order = _InOrder()
return [_as_airflow_tool(toolset, name, tool, ctx, in_order) for name,
tool in toolset_tools.items()]
+def _inert_context(deps: Any = None) -> RunContext[Any]:
Review Comment:
```suggestion
def _build_inert_context(deps: Any = None) -> RunContext[Any]:
```
##########
providers/common/ai/src/airflow/providers/common/ai/tools/_from_toolset.py:
##########
@@ -88,12 +93,102 @@ def airflow_tools_from_toolset(toolset:
AbstractToolset[Any], *, deps: Any = Non
:param toolset: The pydantic-ai toolset to expose.
:param deps: Exposed to the toolset as ``ctx.deps``.
"""
- ctx: RunContext[Any] = RunContext(deps=deps, model=TestModel(),
usage=RunUsage(), max_retries=1)
+ ctx = _inert_context(deps)
toolset_tools = run_coroutine_sync(toolset.get_tools(ctx))
in_order = _InOrder()
return [_as_airflow_tool(toolset, name, tool, ctx, in_order) for name,
tool in toolset_tools.items()]
+def _inert_context(deps: Any = None) -> RunContext[Any]:
+ return RunContext(deps=deps, model=TestModel(), usage=RunUsage(),
max_retries=1)
+
+
+# The wrappers that only choose which tools the model is offered, and under
what name.
+_NAMING_WRAPPERS = (FilteredToolset, PrefixedToolset, RenamedToolset)
+
+
+def _is_naming_wrapper(
+ toolset: object,
+) -> TypeGuard[FilteredToolset[Any] | PrefixedToolset[Any] |
RenamedToolset[Any]]:
+ # Exact types: the tools reached around a wrapper skip its call_tool,
which a subclass may override.
+ return type(toolset) in _NAMING_WRAPPERS
+
+
+def airflow_tools_of(source: object) -> list[AirflowTool]:
+ """
+ Return the tools of a toolset a framework adapter was given.
+
+ A source with an ``airflow_tools()`` method is asked for them. A toolset
wrapped with
+ ``.filtered()``, ``.prefixed()`` or ``.renamed()``, nested in any order,
gives the innermost
+ toolset's own ``airflow_tools()``, so they behave as they do without the
wrappers. The
+ wrappers' own ``get_tools`` decides which of them the model is offered and
under what name,
+ given the same inert ``RunContext`` :func:`airflow_tools_from_toolset`
passes. A ``.filtered()``
+ function therefore runs once, when the adapter is built, rather than
before every model turn
+ as in a pydantic-ai run.
+
+ :raises TypeError: when ``source`` is any other wrapper, such as
``.approval_required()``,
+ or there is no ``airflow_tools()`` method under its wrappers.
+ :raises ValueError: when a ``.renamed()`` map names a tool the toolset
does not have.
Review Comment:
Same wording as in collect_tools: `:raises ValueError:` when a `.renamed()`
map names a tool the toolset does not have. Let's keep the two docstrings in
sync after updating
##########
providers/common/ai/tests/unit/common/ai/tools/test_tools.py:
##########
@@ -153,6 +165,183 @@ def airflow_tools(self) -> list[AirflowTool]:
with pytest.raises(NotImplementedError, match="^not yet$"):
_collect_tools([_Unfinished()], mcp_hint="Connect Acme's McpClient
instead.")
+ def test_a_mock_toolset_is_asked_for_its_tools(self):
+ """The mock has ``airflow_tools`` only through ``__getattr__``, which
a Protocol check misses on Python 3.12+."""
+ source = MagicMock(spec=["airflow_tools"])
+ source.airflow_tools.return_value = [_named("a")]
+
+ assert [tool.name for tool in collect_tools([source])] == ["a"]
+
+
+def _sql_toolset(name: str = "Ada") -> SQLToolset:
+ ts = SQLToolset("pg_default")
+ ts._hook = _make_mock_db_hook(records=[(1, name)],
last_description=[("id",), ("name",)])
+ return ts
+
+
+def _names(tools: list[AirflowTool]) -> list[str]:
+ return [tool.name for tool in tools]
+
+
+def _not_query(ctx: RunContext[Any], tool: ToolDefinition) -> bool:
Review Comment:
```suggestion
def _tool_name_is_not_query(ctx: RunContext[Any], tool: ToolDefinition) ->
bool:
```
##########
providers/common/ai/tests/unit/common/ai/tools/test_tools.py:
##########
@@ -153,6 +165,183 @@ def airflow_tools(self) -> list[AirflowTool]:
with pytest.raises(NotImplementedError, match="^not yet$"):
_collect_tools([_Unfinished()], mcp_hint="Connect Acme's McpClient
instead.")
+ def test_a_mock_toolset_is_asked_for_its_tools(self):
+ """The mock has ``airflow_tools`` only through ``__getattr__``, which
a Protocol check misses on Python 3.12+."""
+ source = MagicMock(spec=["airflow_tools"])
+ source.airflow_tools.return_value = [_named("a")]
+
+ assert [tool.name for tool in collect_tools([source])] == ["a"]
+
+
+def _sql_toolset(name: str = "Ada") -> SQLToolset:
+ ts = SQLToolset("pg_default")
+ ts._hook = _make_mock_db_hook(records=[(1, name)],
last_description=[("id",), ("name",)])
+ return ts
+
+
+def _names(tools: list[AirflowTool]) -> list[str]:
Review Comment:
```suggestion
def _extract_names(tools: list[AirflowTool]) -> list[str]:
```
##########
providers/common/ai/src/airflow/providers/common/ai/tools/_from_toolset.py:
##########
@@ -88,12 +93,102 @@ def airflow_tools_from_toolset(toolset:
AbstractToolset[Any], *, deps: Any = Non
:param toolset: The pydantic-ai toolset to expose.
:param deps: Exposed to the toolset as ``ctx.deps``.
"""
- ctx: RunContext[Any] = RunContext(deps=deps, model=TestModel(),
usage=RunUsage(), max_retries=1)
+ ctx = _inert_context(deps)
toolset_tools = run_coroutine_sync(toolset.get_tools(ctx))
in_order = _InOrder()
return [_as_airflow_tool(toolset, name, tool, ctx, in_order) for name,
tool in toolset_tools.items()]
+def _inert_context(deps: Any = None) -> RunContext[Any]:
+ return RunContext(deps=deps, model=TestModel(), usage=RunUsage(),
max_retries=1)
+
+
+# The wrappers that only choose which tools the model is offered, and under
what name.
+_NAMING_WRAPPERS = (FilteredToolset, PrefixedToolset, RenamedToolset)
+
+
+def _is_naming_wrapper(
+ toolset: object,
+) -> TypeGuard[FilteredToolset[Any] | PrefixedToolset[Any] |
RenamedToolset[Any]]:
+ # Exact types: the tools reached around a wrapper skip its call_tool,
which a subclass may override.
+ return type(toolset) in _NAMING_WRAPPERS
+
+
+def airflow_tools_of(source: object) -> list[AirflowTool]:
+ """
+ Return the tools of a toolset a framework adapter was given.
+
+ A source with an ``airflow_tools()`` method is asked for them. A toolset
wrapped with
+ ``.filtered()``, ``.prefixed()`` or ``.renamed()``, nested in any order,
gives the innermost
+ toolset's own ``airflow_tools()``, so they behave as they do without the
wrappers. The
+ wrappers' own ``get_tools`` decides which of them the model is offered and
under what name,
+ given the same inert ``RunContext`` :func:`airflow_tools_from_toolset`
passes. A ``.filtered()``
+ function therefore runs once, when the adapter is built, rather than
before every model turn
+ as in a pydantic-ai run.
+
+ :raises TypeError: when ``source`` is any other wrapper, such as
``.approval_required()``,
+ or there is no ``airflow_tools()`` method under its wrappers.
+ :raises ValueError: when a ``.renamed()`` map names a tool the toolset
does not have.
+ """
+ # Looked up, not checked against ToolProvider: since Python 3.12 a
Protocol check misses a
+ # method that a mock or proxy only provides through __getattr__.
+ own = getattr(source, "airflow_tools", None)
+ if callable(own):
+ return list(own())
+ if not _is_naming_wrapper(source):
+ raise TypeError(_cannot_take(source))
+ renames: list[PrefixedToolset[Any] | RenamedToolset[Any]] = []
+ inner: AbstractToolset[Any] = source
+ while _is_naming_wrapper(inner):
+ if not isinstance(inner, FilteredToolset):
+ renames.append(inner)
+ inner = inner.wrapped
+ provide = getattr(inner, "airflow_tools", None)
+ if not callable(provide):
+ raise TypeError(_cannot_take(inner))
+ by_name = {tool.name: tool for tool in provide()}
+ offered = run_coroutine_sync(source.get_tools(_inert_context()))
+ tools = []
+ for name in offered:
+ # Map the offered name back through each wrapper, as its ``call_tool``
does.
+ original = name
+ for wrapper in renames:
+ if isinstance(wrapper, PrefixedToolset):
+ original = original.removeprefix(f"{wrapper.prefix}_")
+ else:
+ original = wrapper.name_map.get(original, original)
+ if original not in by_name:
+ raise ValueError(
+ f"The tool offered as {name!r} maps back to {original!r},
which "
+ f"{type(inner).__name__} does not have. .renamed() maps each
new name to the "
+ "toolset's own name, as in .renamed({'run_sql': 'query'})."
+ )
+ tools.append(replace(by_name[original], name=name))
+ return tools
+
+
+def _cannot_take(toolset: object) -> str:
+ kind = type(toolset).__name__
+ if isinstance(toolset, ApprovalRequiredToolset):
+ return (
+ "AirflowTools cannot carry ApprovalRequiredToolset
(.approval_required()) to another "
+ "framework's agent: only a Pydantic AI run pauses for the approval
it asks for. Pass the "
+ "toolset without it. To have a person approve tool calls, run the
agent with "
+ "AgentOperator; a framework's own tool approval, such as
needs_approval in the OpenAI "
+ "Agents SDK, stops the run and nothing in Airflow resumes it."
+ )
Review Comment:
```suggestion
if isinstance(toolset, ApprovalRequiredToolset):
return (
"AirflowTools cannot carry ApprovalRequiredToolset
(.approval_required()) to another "
"framework's agent: only a Pydantic AI run pauses for the
approval it asks for. Pass the "
"toolset without it. To have a person approve tool calls, run
the agent with "
"AgentOperator; a framework's own tool approval, such as
needs_approval in the OpenAI "
"Agents SDK, stops the run and nothing in Airflow resumes it."
)
kind = type(toolset).__name__
```
##########
providers/common/ai/tests/unit/common/ai/tools/test_tools.py:
##########
@@ -153,6 +165,183 @@ def airflow_tools(self) -> list[AirflowTool]:
with pytest.raises(NotImplementedError, match="^not yet$"):
_collect_tools([_Unfinished()], mcp_hint="Connect Acme's McpClient
instead.")
+ def test_a_mock_toolset_is_asked_for_its_tools(self):
+ """The mock has ``airflow_tools`` only through ``__getattr__``, which
a Protocol check misses on Python 3.12+."""
+ source = MagicMock(spec=["airflow_tools"])
+ source.airflow_tools.return_value = [_named("a")]
+
+ assert [tool.name for tool in collect_tools([source])] == ["a"]
+
+
+def _sql_toolset(name: str = "Ada") -> SQLToolset:
Review Comment:
```suggestion
def _build_sql_toolset(name: str = "Ada") -> SQLToolset:
```
##########
providers/common/ai/tests/unit/common/ai/tools/test_tools.py:
##########
@@ -153,6 +165,183 @@ def airflow_tools(self) -> list[AirflowTool]:
with pytest.raises(NotImplementedError, match="^not yet$"):
_collect_tools([_Unfinished()], mcp_hint="Connect Acme's McpClient
instead.")
+ def test_a_mock_toolset_is_asked_for_its_tools(self):
+ """The mock has ``airflow_tools`` only through ``__getattr__``, which
a Protocol check misses on Python 3.12+."""
+ source = MagicMock(spec=["airflow_tools"])
+ source.airflow_tools.return_value = [_named("a")]
+
+ assert [tool.name for tool in collect_tools([source])] == ["a"]
+
+
+def _sql_toolset(name: str = "Ada") -> SQLToolset:
+ ts = SQLToolset("pg_default")
+ ts._hook = _make_mock_db_hook(records=[(1, name)],
last_description=[("id",), ("name",)])
+ return ts
+
+
+def _names(tools: list[AirflowTool]) -> list[str]:
+ return [tool.name for tool in tools]
+
+
+def _not_query(ctx: RunContext[Any], tool: ToolDefinition) -> bool:
+ return tool.name != "query"
+
+
+async def _not_query_async(ctx: RunContext[Any], tool: ToolDefinition) -> bool:
Review Comment:
```suggestion
async def _tool_name_is_not_query_async(ctx: RunContext[Any], tool:
ToolDefinition) -> bool:
```
##########
providers/common/ai/src/airflow/providers/common/ai/tools/_from_toolset.py:
##########
@@ -88,12 +93,102 @@ def airflow_tools_from_toolset(toolset:
AbstractToolset[Any], *, deps: Any = Non
:param toolset: The pydantic-ai toolset to expose.
:param deps: Exposed to the toolset as ``ctx.deps``.
"""
- ctx: RunContext[Any] = RunContext(deps=deps, model=TestModel(),
usage=RunUsage(), max_retries=1)
+ ctx = _inert_context(deps)
toolset_tools = run_coroutine_sync(toolset.get_tools(ctx))
in_order = _InOrder()
return [_as_airflow_tool(toolset, name, tool, ctx, in_order) for name,
tool in toolset_tools.items()]
+def _inert_context(deps: Any = None) -> RunContext[Any]:
+ return RunContext(deps=deps, model=TestModel(), usage=RunUsage(),
max_retries=1)
+
+
+# The wrappers that only choose which tools the model is offered, and under
what name.
+_NAMING_WRAPPERS = (FilteredToolset, PrefixedToolset, RenamedToolset)
+
+
+def _is_naming_wrapper(
+ toolset: object,
+) -> TypeGuard[FilteredToolset[Any] | PrefixedToolset[Any] |
RenamedToolset[Any]]:
+ # Exact types: the tools reached around a wrapper skip its call_tool,
which a subclass may override.
+ return type(toolset) in _NAMING_WRAPPERS
+
+
+def airflow_tools_of(source: object) -> list[AirflowTool]:
Review Comment:
```suggestion
def resolve_airflow_tools(source: object) -> list[AirflowTool]:
```
##########
providers/common/ai/src/airflow/providers/common/ai/tools/_from_toolset.py:
##########
@@ -88,12 +93,102 @@ def airflow_tools_from_toolset(toolset:
AbstractToolset[Any], *, deps: Any = Non
:param toolset: The pydantic-ai toolset to expose.
:param deps: Exposed to the toolset as ``ctx.deps``.
"""
- ctx: RunContext[Any] = RunContext(deps=deps, model=TestModel(),
usage=RunUsage(), max_retries=1)
+ ctx = _inert_context(deps)
toolset_tools = run_coroutine_sync(toolset.get_tools(ctx))
in_order = _InOrder()
return [_as_airflow_tool(toolset, name, tool, ctx, in_order) for name,
tool in toolset_tools.items()]
+def _inert_context(deps: Any = None) -> RunContext[Any]:
+ return RunContext(deps=deps, model=TestModel(), usage=RunUsage(),
max_retries=1)
+
+
+# The wrappers that only choose which tools the model is offered, and under
what name.
+_NAMING_WRAPPERS = (FilteredToolset, PrefixedToolset, RenamedToolset)
+
+
+def _is_naming_wrapper(
+ toolset: object,
+) -> TypeGuard[FilteredToolset[Any] | PrefixedToolset[Any] |
RenamedToolset[Any]]:
+ # Exact types: the tools reached around a wrapper skip its call_tool,
which a subclass may override.
+ return type(toolset) in _NAMING_WRAPPERS
+
+
+def airflow_tools_of(source: object) -> list[AirflowTool]:
+ """
+ Return the tools of a toolset a framework adapter was given.
+
+ A source with an ``airflow_tools()`` method is asked for them. A toolset
wrapped with
+ ``.filtered()``, ``.prefixed()`` or ``.renamed()``, nested in any order,
gives the innermost
+ toolset's own ``airflow_tools()``, so they behave as they do without the
wrappers. The
+ wrappers' own ``get_tools`` decides which of them the model is offered and
under what name,
+ given the same inert ``RunContext`` :func:`airflow_tools_from_toolset`
passes. A ``.filtered()``
+ function therefore runs once, when the adapter is built, rather than
before every model turn
+ as in a pydantic-ai run.
+
+ :raises TypeError: when ``source`` is any other wrapper, such as
``.approval_required()``,
+ or there is no ``airflow_tools()`` method under its wrappers.
+ :raises ValueError: when a ``.renamed()`` map names a tool the toolset
does not have.
+ """
+ # Looked up, not checked against ToolProvider: since Python 3.12 a
Protocol check misses a
+ # method that a mock or proxy only provides through __getattr__.
+ own = getattr(source, "airflow_tools", None)
+ if callable(own):
+ return list(own())
+ if not _is_naming_wrapper(source):
+ raise TypeError(_cannot_take(source))
+ renames: list[PrefixedToolset[Any] | RenamedToolset[Any]] = []
+ inner: AbstractToolset[Any] = source
+ while _is_naming_wrapper(inner):
+ if not isinstance(inner, FilteredToolset):
+ renames.append(inner)
+ inner = inner.wrapped
+ provide = getattr(inner, "airflow_tools", None)
+ if not callable(provide):
+ raise TypeError(_cannot_take(inner))
+ by_name = {tool.name: tool for tool in provide()}
+ offered = run_coroutine_sync(source.get_tools(_inert_context()))
+ tools = []
+ for name in offered:
+ # Map the offered name back through each wrapper, as its ``call_tool``
does.
+ original = name
+ for wrapper in renames:
+ if isinstance(wrapper, PrefixedToolset):
+ original = original.removeprefix(f"{wrapper.prefix}_")
+ else:
+ original = wrapper.name_map.get(original, original)
+ if original not in by_name:
+ raise ValueError(
+ f"The tool offered as {name!r} maps back to {original!r},
which "
+ f"{type(inner).__name__} does not have. .renamed() maps each
new name to the "
+ "toolset's own name, as in .renamed({'run_sql': 'query'})."
+ )
+ tools.append(replace(by_name[original], name=name))
+ return tools
+
+
+def _cannot_take(toolset: object) -> str:
Review Comment:
```suggestion
def _describe_unsupported_toolset(toolset: object) -> str:
```
##########
providers/common/ai/tests/unit/common/ai/tools/test_tools.py:
##########
@@ -153,6 +165,183 @@ def airflow_tools(self) -> list[AirflowTool]:
with pytest.raises(NotImplementedError, match="^not yet$"):
_collect_tools([_Unfinished()], mcp_hint="Connect Acme's McpClient
instead.")
+ def test_a_mock_toolset_is_asked_for_its_tools(self):
Review Comment:
delete
--
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]
