Skip to content

Conversation

vladkolotvin
Copy link

This change allows connection to local database deployed to docker container

This change allows connection to local database deployed to docker container
@rekby
Copy link
Member

rekby commented Sep 3, 2025

Hello, thanks for the PR :)

Why do you need to disable discovery?

@vladkolotvin
Copy link
Author

vladkolotvin commented Sep 8, 2025

@rekby

This PR adds disable_discovery=True when creating the YDB driver so MCP “just works” in the most common developer setups (local YDB in Docker; client either on the host or in a sibling container). Endpoint discovery routinely advertises addresses that are unreachable from those contexts, causing confusing connection failures. Disabling discovery forces the SDK to use the single, known-good endpoint the user configured.

The problem this fixes (reproducible)

When YDB runs in Docker, discovery returns node addresses based on the container’s hostname (often a random container ID or localhost). Those names resolve only inside that container/network, so clients on the host or in another container try to connect to endpoints they can’t reach. There are multiple public reports of this exact failure mode in containerized environments. Stack Overflow

This is amplified by YDB’s own quickstart recipes, which commonly run local YDB with -h localhost. Discovery then advertises localhost—fine inside that container, but wrong for a sibling container where localhost points to itself, not YDB. Result: hard-to-diagnose timeouts until you learn about the discovery toggle.

Why disabling discovery is the safest default for MCP

  • Correctness across environments. With discovery off, the SDK sticks to the endpoint the user already exposed (e.g., localhost:2136 or a compose service name). That endpoint is reachable from host and typical compose networks, removing a whole class of “works only inside this one container” failures.
  • MCP usage pattern. MCP isn’t a long-lived, high-throughput production service. For local single-node YDB and for Serverless/Managed YDB behind a load balancer, discovery mainly optimizes hop selection; it’s not required for correctness in typical MCP workflows. (And if someone truly needs cross-node failover via discovery, see “Configurability” below.)

Trade-offs and impact

  • Local dev: Big usability win—out-of-the-box connectivity without Docker networking footguns. This reduces “time to first successful query” and support load.
  • Cloud/prod: You lose automatic re-discovery if a particular node becomes unavailable. That is acceptable for MCP’s default posture; users who need discovery can opt in.

Configurability (if you prefer a switch)

If you’d rather keep discovery available, I’m happy to add a config flag/env var, e.g. MCP_YDB_DISABLE_DISCOVERY=true|false with default = true. That preserves the friction-free local experience while letting advanced users re-enable discovery explicitly. (The Python SDK’s disable_discovery is a first-class knob, so wiring this is straightforward.)

Alternatives considered (and why they’re weaker defaults)

  • Document “run YDB with -h localhost / tweak Docker networking.” This still breaks for host↔container and container↔container combos and asks newcomers to learn Docker networking before they can run MCP.
  • Auto-detect Docker and switch behavior. Fragile and environment-specific; a single, explicit default is clearer and more reliable.

Disabling discovery by default makes MCP reliable in real-world developer environments with virtually no downside for this tool. Users who need discovery can turn it back on via a flag/env var.

@vgvoleg
Copy link
Collaborator

vgvoleg commented Sep 8, 2025

I'm not sure why, but connection.py do nothing - all connection logic is described in server.py. Probably we forgot to delete this file. Please add a flag here and pass it here

ok let's make it disabled by default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants