sudden code review
This commit is contained in:
@@ -130,9 +130,7 @@ class CombinedCatalog:
|
|||||||
"description": entry.description,
|
"description": entry.description,
|
||||||
"outcomes": list(entry.outcomes),
|
"outcomes": list(entry.outcomes),
|
||||||
"input_schema": entry.input_schema,
|
"input_schema": entry.input_schema,
|
||||||
"output_schema": workflow_output_schema_from_mcp_tool_schema(
|
"output_schema": entry.output_schema,
|
||||||
entry.output_schema
|
|
||||||
),
|
|
||||||
}
|
}
|
||||||
for entry in self.entries()
|
for entry in self.entries()
|
||||||
],
|
],
|
||||||
|
|||||||
@@ -49,9 +49,8 @@ from ...models import (
|
|||||||
ConnectionConfig,
|
ConnectionConfig,
|
||||||
RawWorkflowPlan,
|
RawWorkflowPlan,
|
||||||
)
|
)
|
||||||
from ...runtime import ToolExecutor
|
|
||||||
from ...sdk.converters import workflow_output_schema_from_mcp_tool_schema
|
|
||||||
from ...sdk import BackendAdapter
|
from ...sdk import BackendAdapter
|
||||||
|
from ...runtime import ToolExecutor
|
||||||
from ...shared.errors import error_payload
|
from ...shared.errors import error_payload
|
||||||
from ...shared.names import RESERVED_CONNECTION_IDS
|
from ...shared.names import RESERVED_CONNECTION_IDS
|
||||||
from ...storage import Store
|
from ...storage import Store
|
||||||
@@ -876,7 +875,7 @@ class WfMcpService:
|
|||||||
"""
|
"""
|
||||||
model_prefix = entry.qualified_name.replace(".", "_").replace("-", "_")
|
model_prefix = entry.qualified_name.replace(".", "_").replace("-", "_")
|
||||||
input_model = _model_from_schema(f"{model_prefix}_Input", entry.input_schema)
|
input_model = _model_from_schema(f"{model_prefix}_Input", entry.input_schema)
|
||||||
output_schema = workflow_output_schema_from_mcp_tool_schema(entry.output_schema)
|
output_schema = entry.output_schema
|
||||||
output_model = _model_from_schema(f"{model_prefix}_Output", output_schema)
|
output_model = _model_from_schema(f"{model_prefix}_Output", output_schema)
|
||||||
|
|
||||||
async def invoke_tool(payload: BaseModel) -> NodeReturn[BaseModel]:
|
async def invoke_tool(payload: BaseModel) -> NodeReturn[BaseModel]:
|
||||||
|
|||||||
@@ -18,6 +18,9 @@ from ..shared.names import ProxyNamespace
|
|||||||
|
|
||||||
ProxyT = TypeVar("ProxyT")
|
ProxyT = TypeVar("ProxyT")
|
||||||
ProxyMountFactory = Callable[[ConnectionConfig, Path], "ProxyMount[ProxyT]"]
|
ProxyMountFactory = Callable[[ConnectionConfig, Path], "ProxyMount[ProxyT]"]
|
||||||
|
# Bound proxy listing so one unresponsive upstream source cannot stall the
|
||||||
|
# whole broker. Eight seconds is intentionally longer than normal local stdio
|
||||||
|
# startup/handshake time, but short enough to make a broken source visible.
|
||||||
_PROXY_LIST_TIMEOUT_SECONDS = 8.0
|
_PROXY_LIST_TIMEOUT_SECONDS = 8.0
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -182,9 +185,10 @@ async def _bounded_proxy_list(
|
|||||||
return await asyncio.wait_for(listing, timeout=timeout_seconds)
|
return await asyncio.wait_for(listing, timeout=timeout_seconds)
|
||||||
except (TimeoutError, OSError, ConnectionError) as exc:
|
except (TimeoutError, OSError, ConnectionError) as exc:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"Skipping %s for connection %s after listing failure: %s",
|
"Skipping %s for connection %s after %s listing failure: %s",
|
||||||
operation,
|
operation,
|
||||||
connection_id,
|
connection_id,
|
||||||
|
type(exc).__name__,
|
||||||
exc,
|
exc,
|
||||||
)
|
)
|
||||||
return []
|
return []
|
||||||
|
|||||||
@@ -13,7 +13,6 @@ from wf_mcp.broker.events import McpEvent, make_event
|
|||||||
from ..capabilities import DiscoveredTool
|
from ..capabilities import DiscoveredTool
|
||||||
from ..models import AuthRecord, ConnectionConfig
|
from ..models import AuthRecord, ConnectionConfig
|
||||||
from ..runtime import ToolExecutor
|
from ..runtime import ToolExecutor
|
||||||
from ..sdk.converters import workflow_output_schema_from_mcp_tool_schema
|
|
||||||
|
|
||||||
|
|
||||||
_JSON_TYPE_MAP: dict[str, object] = {
|
_JSON_TYPE_MAP: dict[str, object] = {
|
||||||
@@ -129,10 +128,9 @@ def wrap_discovered_tool(
|
|||||||
f"{connection.id}_{tool.name}_Input",
|
f"{connection.id}_{tool.name}_Input",
|
||||||
tool.input_schema,
|
tool.input_schema,
|
||||||
)
|
)
|
||||||
output_schema = workflow_output_schema_from_mcp_tool_schema(tool.output_schema)
|
|
||||||
output_model = _model_from_schema(
|
output_model = _model_from_schema(
|
||||||
f"{connection.id}_{tool.name}_Output",
|
f"{connection.id}_{tool.name}_Output",
|
||||||
output_schema,
|
tool.output_schema,
|
||||||
)
|
)
|
||||||
|
|
||||||
async def invoke_tool(
|
async def invoke_tool(
|
||||||
@@ -182,5 +180,5 @@ def wrap_discovered_tool(
|
|||||||
description=tool.description,
|
description=tool.description,
|
||||||
is_async=True,
|
is_async=True,
|
||||||
input_schema_contract=tool.input_schema,
|
input_schema_contract=tool.input_schema,
|
||||||
output_schema_contract=output_schema,
|
output_schema_contract=tool.output_schema,
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
import json
|
import json
|
||||||
|
import logging
|
||||||
import sys
|
import sys
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
@@ -142,6 +143,29 @@ def test_proxy_listing_degrades_when_one_source_has_transport_error() -> None:
|
|||||||
asyncio.run(run_failure())
|
asyncio.run(run_failure())
|
||||||
|
|
||||||
|
|
||||||
|
def test_proxy_listing_degrades_when_one_source_has_connection_error(
|
||||||
|
caplog: pytest.LogCaptureFixture,
|
||||||
|
) -> None:
|
||||||
|
async def connection_failed_listing() -> list[Any]:
|
||||||
|
raise ConnectionError("connection refused")
|
||||||
|
|
||||||
|
async def run_failure() -> None:
|
||||||
|
result = await _bounded_proxy_list(
|
||||||
|
connection_failed_listing(),
|
||||||
|
connection_id="serena.default",
|
||||||
|
operation="tools/list",
|
||||||
|
timeout_seconds=1,
|
||||||
|
)
|
||||||
|
assert result == []
|
||||||
|
|
||||||
|
with caplog.at_level(logging.WARNING, logger="wf_mcp.proxy.mounts"):
|
||||||
|
asyncio.run(run_failure())
|
||||||
|
|
||||||
|
assert "ConnectionError" in caplog.text
|
||||||
|
assert "serena.default" in caplog.text
|
||||||
|
assert "tools/list" in caplog.text
|
||||||
|
|
||||||
|
|
||||||
def test_proxy_registers_admin_tools_on_local_provider() -> None:
|
def test_proxy_registers_admin_tools_on_local_provider() -> None:
|
||||||
config = BrokerConfig(
|
config = BrokerConfig(
|
||||||
store_root=local_temp_root() / "proxy_local_admin_store",
|
store_root=local_temp_root() / "proxy_local_admin_store",
|
||||||
|
|||||||
Reference in New Issue
Block a user