-
Notifications
You must be signed in to change notification settings - Fork 133
fix: turn atlas-connect-cluster async #343
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
e654962
24298ed
f45b22f
d2f91d5
8eeb786
6c84179
dad0111
d2c54ae
e978c82
54dfd7b
42b3e47
e267b45
28372be
2134f16
57981cb
693d31c
4274f1b
d1b2324
0acc685
a0ce60c
11d3a14
0286a89
1127f4c
0a1e57c
7430d49
bf41f0d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -11,6 +11,7 @@ const EXPIRY_MS = 1000 * 60 * 60 * 12; // 12 hours | |||||||||||||||||||||||||||
function sleep(ms: number): Promise<void> { | ||||||||||||||||||||||||||||
return new Promise((resolve) => setTimeout(resolve, ms)); | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
export class ConnectClusterTool extends AtlasToolBase { | ||||||||||||||||||||||||||||
protected name = "atlas-connect-cluster"; | ||||||||||||||||||||||||||||
protected description = "Connect to MongoDB Atlas cluster"; | ||||||||||||||||||||||||||||
|
@@ -20,9 +21,32 @@ export class ConnectClusterTool extends AtlasToolBase { | |||||||||||||||||||||||||||
clusterName: z.string().describe("Atlas cluster name"), | ||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
protected async execute({ projectId, clusterName }: ToolArgs<typeof this.argsShape>): Promise<CallToolResult> { | ||||||||||||||||||||||||||||
await this.session.disconnect(); | ||||||||||||||||||||||||||||
private async queryConnection( | ||||||||||||||||||||||||||||
projectId: string, | ||||||||||||||||||||||||||||
clusterName: string | ||||||||||||||||||||||||||||
): Promise<"connected" | "disconnected" | "connecting" | "connected-to-other-cluster"> { | ||||||||||||||||||||||||||||
if (!this.session.connectedAtlasCluster) { | ||||||||||||||||||||||||||||
return "disconnected"; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if ( | ||||||||||||||||||||||||||||
this.session.connectedAtlasCluster.projectId !== projectId || | ||||||||||||||||||||||||||||
this.session.connectedAtlasCluster.clusterName !== clusterName | ||||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||||
return "connected-to-other-cluster"; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if (!this.session.serviceProvider) { | ||||||||||||||||||||||||||||
return "connecting"; | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
await this.session.serviceProvider.runCommand("admin", { | ||||||||||||||||||||||||||||
ping: 1, | ||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||
return "connected"; | ||||||||||||||||||||||||||||
|
await this.session.serviceProvider.runCommand("admin", { | |
ping: 1, | |
}); | |
return "connected"; | |
try { | |
await this.session.serviceProvider.runCommand("admin", { | |
ping: 1, | |
}); | |
return "connected"; | |
} catch (error) { | |
logger.warn(LogId.ConnectionPingError, `Ping command failed: ${error.message}`); | |
return "connecting"; | |
} |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need, I'm bubbling up the error
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const connectionString = cn.toString(); | |
return connectionString; | |
return cn.toString(); |
Outdated
Copilot
AI
Jul 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extract the retry count (600) and delay (500ms) into named constants to improve readability and ease future adjustments.
for (let i = 0; i < 600; i++) { | |
// try for 5 minutes | |
for (let i = 0; i < RETRY_COUNT; i++) { | |
// try for RETRY_COUNT attempts |
Copilot uses AI. Check for mistakes.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If those are not set, does it make sense to make that call at all?
fmenezes marked this conversation as resolved.
Show resolved
Hide resolved
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -189,7 +189,23 @@ describeWithAtlas("clusters", (integration) => { | |
})) as CallToolResult; | ||
expect(response.content).toBeArray(); | ||
expect(response.content).toHaveLength(1); | ||
expect(response.content[0]?.text).toContain(`Connected to cluster "${clusterName}"`); | ||
expect(response.content[0]?.type).toEqual("text"); | ||
expect(response.content[0]?.text).toContain(`Attempting to connect to cluster "${clusterName}"...`); | ||
|
||
for (let i = 0; i < 600; i++) { | ||
|
||
const response = (await integration.mcpClient().callTool({ | ||
name: "atlas-connect-cluster", | ||
arguments: { projectId, clusterName }, | ||
})) as CallToolResult; | ||
expect(response.content).toBeArray(); | ||
expect(response.content).toHaveLength(1); | ||
expect(response.content[0]?.type).toEqual("text"); | ||
const c = response.content[0] as { text: string }; | ||
if (c.text.includes("Cluster is already connected.")) { | ||
break; // success | ||
} | ||
await sleep(500); | ||
} | ||
}); | ||
}); | ||
}); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The log ID key
atlasConnectSuccessed
is misspelled; consider renaming it toatlasConnectSucceeded
oratlasConnectSuccess
for clarity.Copilot uses AI. Check for mistakes.