-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
language_model_selector: Authenticate all providers up front #25123
Merged
maxdeviant
merged 3 commits into
main
from
marshall/authenticate-all-language-model-providers
Feb 19, 2025
Merged
language_model_selector: Authenticate all providers up front #25123
maxdeviant
merged 3 commits into
main
from
marshall/authenticate-all-language-model-providers
Feb 19, 2025
Conversation
This file contains 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
maxdeviant
added a commit
that referenced
this pull request
Feb 19, 2025
…er::authenticate` (#25126) This PR updates the `LanguageModelProvider::authenticate` method to return an `AuthenticateError` instead of an `anyhow::Error`. This allows us to model the "credentials not found" state explicitly as `AuthenticateError::CredentialsNotFound`, which enables the caller to check for this state and act accordingly. Planning to use this in #25123 to silence errors about missing credentials when authenticating providers in the background. Release Notes: - N/A
223fd9f
to
34eea65
Compare
osiewicz
pushed a commit
to RemcoSmitsDev/zed
that referenced
this pull request
Feb 19, 2025
…er::authenticate` (zed-industries#25126) This PR updates the `LanguageModelProvider::authenticate` method to return an `AuthenticateError` instead of an `anyhow::Error`. This allows us to model the "credentials not found" state explicitly as `AuthenticateError::CredentialsNotFound`, which enables the caller to check for this state and act accordingly. Planning to use this in zed-industries#25123 to silence errors about missing credentials when authenticating providers in the background. Release Notes: - N/A
osiewicz
pushed a commit
to RemcoSmitsDev/zed
that referenced
this pull request
Feb 19, 2025
…ustries#25123) This PR fixes an issue where configured language model providers would not show up unless the configuration view was opened. The problem was that we were filtering unauthenticated language model providers out of the language model selector, but would only authenticate the active provider when the selector loaded. Authenticating the rest of the providers was deferred until the configuration view was opened for the first time. Closes zed-industries#21821. Release Notes: - Fixed an issue where configured languages models were not showing up in the language model selector until the configuration view was opened for the first time.
maxdeviant
added a commit
that referenced
this pull request
Feb 20, 2025
…25266) This PR adds a new `CredentialsProvider` trait that abstracts over interacting with the system keychain. We had previously introduced a version of this scoped just to Zed auth in #11505. However, after landing #25123, we now have a similar issue with the credentials for language model providers that are also stored in the keychain (and thus also produce a spam of popups when running a development build of Zed). This PR takes the existing approach and makes it more generic, such that we can use it everywhere that we need to read/store credentials in the keychain. There are still two credential provider implementations: - `KeychainCredentialsProvider` will interact with the system keychain (using the existing GPUI APIs) - `DevelopmentCredentialsProvider` will use a local file on the file system We only use the `DevelopmentCredentialsProvider` when: 1. We are running a development build of Zed 2. The `ZED_DEVELOPMENT_AUTH` environment variable is set - I am considering removing the need for this and making it the default, but that will be explored in a follow-up PR. Release Notes: - N/A
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR fixes an issue where configured language model providers would not show up unless the configuration view was opened.
The problem was that we were filtering unauthenticated language model providers out of the language model selector, but would only authenticate the active provider when the selector loaded. Authenticating the rest of the providers was deferred until the configuration view was opened for the first time.
Closes #21821.
Release Notes: