-
Notifications
You must be signed in to change notification settings - Fork 1.6k
adopt to new way of configuring BYOK models #2782
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
Conversation
|
Does each model now require a vendor config to be explicitly defined (for e.g. google)? Also, any reason the test cannot be updated for gemini in this PR? |
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.
Pull request overview
This PR adopts a new way of configuring BYOK (Bring Your Own Key) models by moving from custom API key management UI to declarative configuration in package.json. The changes include:
Changes:
- Deprecated old configuration settings (
azureAuthType,azureModels,customOAIModels,ollamaEndpoint) and moved them to aDeprecatednamespace - Added configuration schemas to language model chat provider declarations in package.json for all BYOK vendors
- Refactored all provider implementations to use the new abstract base classes and configuration system
- Removed custom UI for API key management (commands, configurator, UI service)
- Added migration logic to automatically move old configurations to new format
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Added configuration schemas for BYOK providers; removed management commands |
| package.nls.json | Removed localization strings for deprecated settings |
| configurationServiceImpl.ts | Enhanced setConfig to handle undefined values across all configuration targets |
| configurationService.ts | Added Deprecated namespace for old settings |
| vscode.proposed.chatProvider.d.ts | Added configuration support to PrepareLanguageModelChatModelOptions |
| abstractLanguageModelChatProvider.ts | New base classes for language model chat providers |
| All provider files | Refactored to extend new base classes and use configuration-based approach |
| byokContribution.ts | Removed command registrations for API key management |
| Deleted files | Removed custom UI components (configurator, UI service, base provider) |
| Test files | Skipped Gemini tests; removed Azure provider tests; updated configuration tests |
Comments suppressed due to low confidence (1)
package.json:1964
- The 'gpu' property has an incorrect description that states "Display name of the custom OpenAI model", which appears to be a copy-paste error. This is a duplicate of the same issue in the customoai configuration. Either correct the description to explain what the gpu property is for, or remove this property if it's not used.
"gpu": {
"type": "string",
"description": "Display name of the custom OpenAI model"
},
src/extension/byok/vscode-node/abstractLanguageModelChatProvider.ts
Outdated
Show resolved
Hide resolved
lramos15
left a comment
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.
Please take a look at the copilot spelling mistakes and fix those too thanks!
@lramos15 Resolved all feedback |
|
CI is red |
@lramos15 adopted to new way of configuring BYOK models - microsoft/vscode#286616
Vendors requiring API key
Adopted Anthropic, xAI, Google, Groq, OpenRouter and OpenAI vendors to declare the required
apiKeysecret as part of the configuration of respective languageModelChatProviders contributions.cc @bhavyaus (Anthropic), @vijayupadya (Google)
@vijayupadya - Skipped gemini spec tests. Please update them as per the latest adoption.
Example:
{ "vendor": "anthropic", "displayName": "Anthropic", "configuration": { "properties": { "apiKey": { "type": "string", "secret": true, "description": "API key for Anthropic" } }, "required": [ "apiKey" ] } }Vendors requiring end point
Adopted Ollama vendor to declare the required Ollama endpoint as part of the configuration of respective languageModelChatProviders contributions.
{ "vendor": "ollama", "displayName": "Ollama", "configuration": { "type": "object", "properties": { "url": { "type": "string", "description": "The endpoint URL for the Ollama server", "default": "http://localhost:11434" } }, "required": [ "url" ] } }Open AI Compatible & Azure vendors
Adopted OpenAI Compatible and Azure vendors that requires users to provide the models configuration in the settings to use the new format. This configuration is now declared as part of the languageModelChantProviders declaration - package.json
cc @eleanorjboyd