Speed up Cloudflare sharing.#2420
Open
hynek-urban wants to merge 2 commits into
Open
Conversation
Cloudflare sharing in minds fired 'mngr imbue_cloud tunnels list' to find
an agent's tunnel, which enumerated every tunnel on the account and fetched
each one's config server-side (O(n) Cloudflare calls) -- 8-11s on accounts
with many tunnels.
minds always knows the exact tunnel name it wants
(<username>--<agent-prefix>), so add:
- Connector: GET /tunnels/by-agent/{agent_id} + ForwardingCtx.get_tunnel_for_agent,
resolving the tunnel via Cloudflare's server-side name filter plus one config
fetch (2 Cloudflare calls regardless of N); 404 when absent.
- mngr_imbue_cloud: 'tunnels find-by-agent' CLI + client.find_tunnel_for_agent
(emits/parses null when absent).
- minds: point ImbueCloudCli.find_tunnel_for_agent at the new lookup.
…ndpoint
Minds clients update independently of (and often ahead of) the deployed
connector, so a client with the new 'tunnels find-by-agent' code can hit a
connector that lacks GET /tunnels/by-agent/{agent_id}. The endpoint's
unknown-route 404 was being mapped to 'no tunnel', so sharing status was
stuck at enabled=false against an un-redeployed connector.
- Connector: return HTTP 200 + null for 'no tunnel' (reserving 404 for
'endpoint absent').
- Client: on 404, transparently fall back to the O(n) GET /tunnels
enumeration (matching the <username>--<agent-prefix> slug); on 200+null,
return None with no fallback. Fast path is unchanged when the tunnel exists.
Contributor
|
this actually looks pretty good to me! I think it'd be a good one for you to test (to learn how to deploy stuff), happy to chat about how to do that |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
After reducing the
mngrcall overhead in #2403, the next big thing in terms of slowness in Cloudflare sharing wasmngr imbue_cloud tunnels list. Apparently, it was unnecessary - on the server side (ForwardingCtx.list_tunnels), it iterated over all existing tunnels and retrieved details for each of them in separate requests to CloudFlare, even though the client only needed the details of a single tunnel.This PR introduces a new command in the CLI and a new endpoint on the Imbue Cloud backend (remote service connector) to only request info about the one tunnel it needs.
The PR consists of two commits. The second one is there just for backwards compatibility - if we don't need new Minds clients to support old backends, I think the second commit can be safely reverted. (I wasn't sure what our deployment strategy is so I included it.)
Disclaimer: This was mostly driven by Claude itself; I'm not really familiar with the parts of the code this touches. It looks and sounds reasonable but I'm not 100% sure and I didn't properly test it because it involves updating the remote backend.