aminghadersohi commented on PR #44200:
URL: https://github.com/apache/superset/pull/44200#issuecomment-5771873276
Ran a QA pass on this before letting it merge downstream. OSS test-wise it's
healthy — `tests/unit_tests` is 4655 passed / 2 skipped under fastmcp 4.0.3 +
mcp 2.2.0, and a live MCP server on this branch handled a full auth matrix
(JWT, API key, revoked key, unknown user, 8 concurrent calls across two
identities) and real tool calls correctly.
But a few behaviour changes ride along that nothing here asserts:
**The loosened assertion is hiding more than a message change.**
`match="Unknown resource"` → `"Unknown resource|Resource not found"` covers one
of four changes. Measured side by side on identical probe code:
| | 3.4.7 / 1.29.1 | 4.0.3 / 2.2.0 |
|---|---|---|
| message | `Resource not found: Unknown resource: '<uri>'` | `Resource not
found: '<uri>'` |
| JSON-RPC code | `-32002` | `-32602` |
| `data` | `None` | `{'uri': '<uri>'}` |
| class | `McpError` | `MCPError` (`fastmcp.exceptions.McpError` kept as
alias) |
`-32002` is resource-not-found, `-32602` is generic invalid-params. Any
client branching on `-32002` stops recognising the case, and the widened regex
makes that invisible to CI. Worth asserting the code explicitly.
**Two HTTP stacks now resolve, with different TLS trust roots.** `httpx`
0.28.1 stays, `httpx2` 2.12.0 + `truststore` arrive. Separate namespaces, no
import collision — but `httpx.Client()` gets an `ssl.SSLContext` on the certifi
bundle, while `httpx2.Client()` gets `truststore._api.SSLContext` on the **OS
trust store**. `fastmcp-slim` and `mcp` are on httpx2, so MCP outbound TLS no
longer follows certifi. Deployments that inject a private CA into certifi or
`REQUESTS_CA_BUNDLE` but not the OS store will see MCP egress fail while the
rest of Superset is fine. No test covers it.
Smaller ones, all reproduced:
- Custom (non-standard) tool annotations are silently stripped from the tool
definition in fastmcp 4.
- `Annotated[int, Field(strict=True)]` on a tool arg is no longer enforced —
`"2"` used to error, now coerces to `2`.
- A tool annotated `-> list` (bare) loses
`outputSchema`/`structuredContent`. This is the flip side of the fix for the
double-image/output-validation bug, but it's a contract change for clients
consuming structured output.
- `code: 0` shows up as a JSON-RPC error code on unknown methods (fastmcp 4)
and on unknown resource URIs (fastmcp 3) — not a valid code on either version,
and Superset's error middleware masks the clean codes into a generic internal
error.
Additive on the wire: `CallToolResult.resultType`,
`io.modelcontextprotocol/serverInfo` in `_meta`.
One negative worth recording: the OAuth/DCR surface is purely additive 3.4.7
→ 4.0.3 (new fields all optional with `None` defaults), and a fastmcp-3-era
persisted `ProxyDCRClient` record round-trips under v4 cleanly. No upgrade
landmine there.
Not verified: real OAuth/DCR end-to-end (register → authorize → token →
refresh), integration tests, SSE streaming/resumption, perf. Flagging so nobody
reads the green CI as covering them.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
