Skip to content
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

ConfigHandler: make updateIdeSettings work (better) #3456

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

owtaylor
Copy link

Description

ConfigHandler.updateIdeSettings() was largely ineffective because the ideSettingsPromise was stored in the ProfileLoader object, so updating to a new ideSettingsPromise resulted in the config being reloaded with the old settings. Fix this by passing the ideSettingsPromise as a parameter to IProfileLoader.doLoadConfig().

I came up with this while working on a larger change (for automatic model selection), so I don't have a great example of when this would matter, but it does seem like this would affect the use of ideSettings.userToken in core/llm/llms/index.ts -

  if (desc.provider === "continue-proxy") {
    options.apiKey = ideSettings.userToken;
    if (ideSettings.remoteConfigServerUrl) {
      options.apiBase = new URL(
        "/proxy/v1",
        ideSettings.remoteConfigServerUrl,
      ).toString();
    }
  }

This patch does not fix the case where changing the settings affects what profiles (changes to ideSettings.remoteConfigServerUrl)

Checklist

  • The relevant docs, if any, have been updated or created
  • The relevant tests, if any, have been updated or created

ConfigHandler.updateIdeSettings() was largely ineffective because the
ideSettingsPromise was stored in the ProfileLoader object, so updating
to a new ideSettingsPromise resulted in the config being reloaded
with the old settings. Fix this by passing the ideSettingsPromise as
a parameter to IProfileLoader.doLoadConfig().
Copy link

netlify bot commented Dec 19, 2024

Deploy Preview for continuedev canceled.

Name Link
🔨 Latest commit 9767cee
🔍 Latest deploy log https://app.netlify.com/sites/continuedev/deploys/676452a868480b000848e4a4

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.

1 participant