-
Notifications
You must be signed in to change notification settings - Fork 2.5k
feat: add Ollama API key support for Turbo mode #7425
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 all commits
2b8cdb7
465dd2f
0db0df5
00628f8
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 |
|---|---|---|
|
|
@@ -25,10 +25,20 @@ export class OllamaHandler extends BaseProvider implements SingleCompletionHandl | |
| super() | ||
| this.options = options | ||
|
|
||
| // Use the API key if provided (for Ollama cloud or authenticated instances) | ||
| // Otherwise use "ollama" as a placeholder for local instances | ||
| const apiKey = this.options.ollamaApiKey || "ollama" | ||
|
|
||
| const headers: Record<string, string> = {} | ||
| if (this.options.ollamaApiKey) { | ||
| headers["Authorization"] = `Bearer ${this.options.ollamaApiKey}` | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is passing the API key in both and necessary? The OpenAI client typically handles authentication via the parameter. The Authorization header in might be redundant unless Ollama's OpenAI-compatible endpoint specifically requires it. Could we test if just using without the works? |
||
| } | ||
|
|
||
| this.client = new OpenAI({ | ||
| baseURL: (this.options.ollamaBaseUrl || "http://localhost:11434") + "/v1", | ||
| apiKey: "ollama", | ||
| apiKey: apiKey, | ||
| timeout: getApiRequestTimeout(), | ||
| defaultHeaders: headers, | ||
| }) | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,6 +86,19 @@ export const Ollama = ({ apiConfiguration, setApiConfigurationField }: OllamaPro | |
| className="w-full"> | ||
| <label className="block font-medium mb-1">{t("settings:providers.ollama.baseUrl")}</label> | ||
| </VSCodeTextField> | ||
| {apiConfiguration?.ollamaBaseUrl && ( | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The conditional rendering logic here is clever - only showing the API key field when a custom base URL is provided. Consider adding a comment explaining this UX decision to help future maintainers: |
||
| <VSCodeTextField | ||
| value={apiConfiguration?.ollamaApiKey || ""} | ||
| type="password" | ||
| onInput={handleInputChange("ollamaApiKey")} | ||
| placeholder={t("settings:placeholders.apiKey")} | ||
| className="w-full"> | ||
| <label className="block font-medium mb-1">{t("settings:providers.ollama.apiKey")}</label> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical issue: The translation keys and referenced here don't exist in the translation files. This will cause the UI to display missing translation keys instead of proper text. You need to add these keys to and all other locale files: |
||
| <div className="text-xs text-vscode-descriptionForeground mt-1"> | ||
| {t("settings:providers.ollama.apiKeyHelp")} | ||
| </div> | ||
| </VSCodeTextField> | ||
| )} | ||
| <VSCodeTextField | ||
| value={apiConfiguration?.ollamaModelId || ""} | ||
| onInput={handleInputChange("ollamaModelId")} | ||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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 test verifies that the handler can be instantiated with an API key, but doesn't actually test that the API key is used in the Authorization header. Could we add an assertion to verify the header is properly set?
Consider mocking the OpenAI constructor to capture and verify the headers: