Skip to content

feat: suggest the config connection string on connect failure #75

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

Merged
merged 1 commit into from
Apr 16, 2025

Conversation

nirinchev
Copy link
Collaborator

If the user has a configured connection string and the connect fails (e.g. due to the model infering an incorrect connection string from the code), suggest using the config connection string instead.

Fixes #73

@nirinchev nirinchev requested a review from gagik April 14, 2025 09:11
Copy link
Collaborator

@gagik gagik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice

Base automatically changed from ni/integration-list-databases to main April 14, 2025 11:48
return {
content: [{ type: "text", text: `Successfully connected to ${connectionString}.` }],
};
} catch (error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: alternatively you can override handleError here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, it's a bit more complicated since I wanted to have the extra check comparing the connection string we tried connecting to to the connection string in the config. handleError right now doesn't supply the original args used to invoke the tool and even if it did, in the case of connecting by cluster name, we wouldn't know what the resolved connection string is.

@nirinchev nirinchev merged commit 8c341bf into main Apr 16, 2025
4 checks passed
@nirinchev nirinchev deleted the ni/connect branch April 16, 2025 08:22
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.

Suggest configured connection string in case of connection failure
3 participants