Skip to content

feat: Vertex AI support #4458

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 26 commits into from
Apr 8, 2025
Merged

feat: Vertex AI support #4458

merged 26 commits into from
Apr 8, 2025

Conversation

raunakab
Copy link
Contributor

@raunakab raunakab commented Apr 4, 2025

Description

This PR adds support for the Gemini/VertexAI LLM Provider.
Addresses https://linear.app/danswer/issue/DAN-1704/built-in-gemini-vertex-ai-support.

How Has This Been Tested?

Primarily UI based changes; tested manually to make sure registration/deletion works.

@raunakab raunakab requested a review from a team as a code owner April 4, 2025 21:09
Copy link

vercel bot commented Apr 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
internal-search ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 7, 2025 11:38pm

@raunakab raunakab changed the title feat: vertex ai support feat: Vertex AI support Apr 4, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

This PR introduces support for the Gemini/VertexAI LLM Provider by updating configuration interfaces, backend completion calls, and UI components to properly handle file-based credentials and streamline provider-specific options.

  • Updated backend/onyx/llm/chat_llm.py to conditionally inject 'vertex_credentials' into the Litellm call for vertex_ai.
  • Extended backend/onyx/llm/interfaces.py by adding an optional 'credentials_file' in LLMConfig.
  • Added Gemini/VertexAI support in backend/onyx/llm/llm_provider_options.py with drag-and-drop custom config keys.
  • Modified web/src/app/admin/configuration/llm/LLMConfiguration.tsx and LLMProviderUpdateForm.tsx for simplified UI handling of VertexAI.

💡 (2/5) Greptile learns from your feedback when you react with 👍/👎!

8 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

Copy link
Contributor

@hagen6835 hagen6835 left a comment

Choose a reason for hiding this comment

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

lgtm

@raunakab raunakab enabled auto-merge April 4, 2025 23:46
disabled={existingLlmProvider ? true : false}
/>
)}
<TextFormField
Copy link
Contributor

Choose a reason for hiding this comment

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

I def agree that hideAdvanced shouldn't control this, but I think we still need a way to hide this. Basically, when a user first comes to the app, we don't want to ask them to put in a name, it slightly increases friction (see ApiKeyForm)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, you're right. I think I was a little bullish with the removal of this field. I've added it back (and renamed it to be a little bit more aligned with its purpose).

I've renamed it to firstTimeConfiguration. Do you think that's an appropriate name here? It's only used in ApiKeyForm, which I believe is only shown during first time configurations?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, that sounds great

@raunakab raunakab force-pushed the feat/vertexai-support branch from 03981de to 8c0bee9 Compare April 6, 2025 00:05
Copy link
Contributor

@Weves Weves left a comment

Choose a reason for hiding this comment

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

On startup, I'm seeing this. Previously there would not be the Default Model section. Would like to keep it that way to keep the initial set up as simple as possible.

Screenshot 2025-04-07 at 1 02 51 PM

disabled={existingLlmProvider ? true : false}
/>
)}
<TextFormField
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, that sounds great

Copy link
Contributor

@Weves Weves left a comment

Choose a reason for hiding this comment

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

Screenshot 2025-04-07 at 3 58 30 PM

When editing, this is the view I see. I believe it should have all the same options as when creating a new one (e.g. be able to change the default model, display models, etc.)

Copy link
Contributor

@Weves Weves left a comment

Choose a reason for hiding this comment

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

lgtm 🍾

@raunakab raunakab added this pull request to the merge queue Apr 8, 2025
Merged via the queue into main with commit 206daa6 Apr 8, 2025
11 checks passed
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.

3 participants