Skip to content

Commit c43896d

Browse files
committed
PR feedback updates
1 parent bcfe0ba commit c43896d

File tree

4 files changed

+25
-17
lines changed

4 files changed

+25
-17
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ client = McpdClient(api_endpoint="http://localhost:8090", api_key="optional-key"
145145

146146
* `client.tools(server_name: str) -> list[dict]` - Returns the tool schema definitions for only the specified server.
147147

148-
* `client.agent_tools(servers: list[str] | None = None, check_health: bool = True) -> list[Callable]` - Returns a list of self-contained, callable functions suitable for agentic frameworks. By default, filters to healthy servers only. Use `servers` to filter by server names, or `check_health=False` to include all servers regardless of health.
148+
* `client.agent_tools(servers: list[str] | None = None, *, check_health: bool = True) -> list[Callable]` - Returns a list of self-contained, callable functions suitable for agentic frameworks. By default, filters to healthy servers only. Use `servers` to filter by server names, or `check_health=False` to include all servers regardless of health.
149149

150150
* `client.clear_agent_tools_cache()` - Clears cached generated callable functions that are created when calling agent_tools().
151151

src/mcpd/mcpd_client.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,8 @@ def agent_tools(self, servers: list[str] | None = None, *, check_health: bool =
394394
Most users should leave this as True for best performance.
395395
396396
Returns:
397-
A list of callable functions, one for each tool from healthy servers.
397+
A list of callable functions, one for each tool from healthy servers (if check_health=True, the default)
398+
or all servers (if check_health=False).
398399
Each function has the following attributes:
399400
- __name__: The tool's qualified name (e.g., "time__get_current_time")
400401
- __doc__: The tool's description
@@ -406,7 +407,6 @@ def agent_tools(self, servers: list[str] | None = None, *, check_health: bool =
406407
ConnectionError: If unable to connect to the mcpd daemon.
407408
TimeoutError: If requests to the daemon time out.
408409
AuthenticationError: If API key authentication fails.
409-
ServerNotFoundError: If a server becomes unavailable during tool retrieval.
410410
McpdError: If unable to retrieve server health status (when check_health=True)
411411
or retrieve tool definitions or generate functions.
412412
@@ -482,6 +482,9 @@ def _get_healthy_servers(self, server_names: list[str]) -> list[str]:
482482
This method silently skips servers that don't exist or have
483483
unhealthy status (timeout, unreachable, unknown).
484484
"""
485+
if not server_names:
486+
return []
487+
485488
health_map = self.server_health()
486489

487490
healthy_servers = [

tests/conftest.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from collections.abc import Callable
12
from unittest.mock import Mock
23

34
import pytest
@@ -73,8 +74,8 @@ def test_something(tools_side_effect):
7374
mock_tools.side_effect = tools_side_effect(tools_map)
7475
"""
7576

76-
def _create_side_effect(tools_map: dict[str, list[dict]]):
77-
def side_effect(server_name=None):
77+
def _create_side_effect(tools_map: dict[str, list[dict]]) -> Callable[[str | None], list[dict]]:
78+
def side_effect(server_name: str | None = None) -> list[dict]:
7879
return tools_map.get(server_name, [])
7980

8081
return side_effect

tests/unit/test_mcpd_client.py

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -227,13 +227,13 @@ def test_agent_tools_with_nonexistent_server(self, mock_tools, client, tools_sid
227227
@patch.object(McpdClient, "server_health")
228228
def test_agent_tools_with_empty_servers_list(self, mock_health, mock_tools, client):
229229
"""Test filtering with empty server list."""
230-
mock_health.return_value = {}
231-
232230
with patch.object(client._function_builder, "create_function_from_schema") as mock_create:
233231
result = client.agent_tools(servers=[])
234232

235233
assert result == []
236-
assert mock_create.call_count == 0
234+
mock_health.assert_not_called()
235+
mock_tools.assert_not_called()
236+
mock_create.assert_not_called()
237237

238238
@patch.object(McpdClient, "servers")
239239
@patch.object(McpdClient, "tools")
@@ -323,13 +323,15 @@ def test_agent_tools_no_health_check_when_disabled(self, mock_tools, mock_server
323323
@patch.object(McpdClient, "servers")
324324
@patch.object(McpdClient, "tools")
325325
@patch.object(McpdClient, "server_health")
326-
def test_agent_tools_all_servers_unhealthy(self, mock_health, mock_tools, mock_servers, client):
326+
def test_agent_tools_all_servers_unhealthy(self, mock_health, mock_tools, mock_servers, client, tools_side_effect):
327327
"""Test behavior when all servers are unhealthy."""
328328
mock_servers.return_value = ["server1", "server2"]
329-
mock_tools.return_value = {
330-
"server1": [{"name": "tool1", "description": "Tool 1"}],
331-
"server2": [{"name": "tool2", "description": "Tool 2"}],
332-
}
329+
mock_tools.side_effect = tools_side_effect(
330+
{
331+
"server1": [{"name": "tool1", "description": "Tool 1"}],
332+
"server2": [{"name": "tool2", "description": "Tool 2"}],
333+
}
334+
)
333335

334336
mock_health.return_value = {
335337
"server1": {"status": "timeout"},
@@ -375,12 +377,14 @@ def test_agent_tools_with_servers_and_health_filtering(self, mock_health, mock_t
375377
@patch.object(McpdClient, "servers")
376378
@patch.object(McpdClient, "tools")
377379
@patch.object(McpdClient, "server_health")
378-
def test_agent_tools_health_check_exception(self, mock_health, mock_tools, mock_servers, client):
380+
def test_agent_tools_health_check_exception(self, mock_health, mock_tools, mock_servers, client, tools_side_effect):
379381
"""Test behavior when health check raises exception."""
380382
mock_servers.return_value = ["server1"]
381-
mock_tools.return_value = {
382-
"server1": [{"name": "tool1", "description": "Tool 1"}],
383-
}
383+
mock_tools.side_effect = tools_side_effect(
384+
{
385+
"server1": [{"name": "tool1", "description": "Tool 1"}],
386+
}
387+
)
384388

385389
# Health check raises exception.
386390
mock_health.side_effect = Exception("Health check failed")

0 commit comments

Comments
 (0)