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

Support Google's Vertex AI #436

Merged
merged 2 commits into from
Nov 15, 2023
Merged

Support Google's Vertex AI #436

merged 2 commits into from
Nov 15, 2023

Conversation

rhyst
Copy link

@rhyst rhyst commented Nov 7, 2023

This PR adds support for Google's Vertex AI which is fairly simple as the litellm package just supports it.

I don't know if the way I dealt with the config is correct, at the moment it seems like everything else needs an Open AI key but obviously Vertex does not so I removed the attribute error check.

If you are open to accepting this PR then I'm happy to update documentation etc. as well.

@alihasnain0786
Copy link

/review

Copy link
Contributor

github-actions bot commented Nov 7, 2023

PR Analysis

(review updated until commit 557ec72)

  • 🎯 Main theme: Adding support for Google's Vertex AI to the project
  • 📝 PR summary: This PR introduces support for Google's Vertex AI in the project. It includes changes in the configuration files to accommodate the new AI platform and updates the AI handler to manage the Vertex AI settings. The PR also updates the documentation and requirements to reflect the new changes.
  • 📌 Type of PR: Enhancement
  • 🧪 Relevant tests added: No
  • ⏱️ Estimated effort to review [1-5]: 3, because the PR involves changes in multiple files and introduces a new AI platform which requires understanding of how it integrates with the existing codebase.
  • 🔒 Security concerns: No security concerns found

PR Feedback

  • 💡 General suggestions: The PR is well-structured and the changes are logically grouped. However, it would be beneficial to include tests that verify the new functionality. Additionally, it would be helpful to ensure that the existing functionality is not broken by these changes.

  • 🤖 Code feedback:

    • relevant file: pr_agent/algo/ai_handler.py
      suggestion: Consider handling the case where neither OPENAI.KEY nor VERTEXAI.VERTEX_PROJECT is provided. Currently, if neither of these keys is provided, the program might run into issues later on. A check at the start can help avoid this. [important]
      relevant line: '+ if get_settings().get("OPENAI.KEY", None):'

    • relevant file: pr_agent/algo/__init__.py
      suggestion: It would be beneficial to add a comment explaining what the numbers associated with each model represent. This would improve code readability and maintainability. [medium]
      relevant line: '+ 'codechat-bison-32k': 32000,'

    • relevant file: pr_agent/algo/ai_handler.py
      suggestion: It might be a good idea to refactor the init method. It's quite long and does a lot of things. Breaking it down into smaller, more manageable methods would improve code readability and maintainability. [medium]
      relevant line: '+ if get_settings().get("VERTEXAI.VERTEX_PROJECT", None):'

    • relevant file: requirements.txt
      suggestion: It's a good practice to pin all the dependencies to a specific version. This can help avoid potential issues caused by updates to the dependencies that might introduce breaking changes. [medium]
      relevant line: '+google-cloud-aiplatform==1.35.0'

@mrT23
Copy link
Collaborator

mrT23 commented Nov 7, 2023

Hi @rhyst
The PR looks quite good. Not everything needs an openai key, we do support other models.

some notes:

  • You should add a template for vertex usage in pr_agent/settings/.secrets_template.toml
  • Some lines of documentation will help a lot to make this feature accessible
  • have you verified that after this commit, you are able to generate feedback (review, describe,...) with vertex AI ?
    and what about openai (i mainly refer to upgrading the litellm version, we had stability issues before) ?

Just for my personal knowledge - what is Vertex AI ? :-)
is it a model ? how good is it ? why don't you need a key to use it ?

@mrT23 mrT23 self-requested a review November 7, 2023 13:00
@mrT23 mrT23 added the enhancement New feature or request label Nov 7, 2023
@rhyst
Copy link
Author

rhyst commented Nov 7, 2023

I'll add some docs 🙂

I have tested the review command on this branch with the Vertex AI models codechat-bison and codechat-bison-32k and also with OpenAI gpt-3.5 and gpt-4. I will verify the other commands as well since you have had issues with litellm in the past.

Vertex AI I think is Google's "AI platform". Within it they have several off the shelf models like chat-bison, codechat-bison, codechat-bison-32k etc. I think you can also access custom models that you make on the Vertex AI platform though I have not used that yet.

In my limited experience so far the codechat-bison models seem to produce similar output to gpt-3.5 when used for code review tasks. I thought gpt-4 was noticeably better but I also estimated that currently Vertex AI models are about 30x cheaper than Open AI models so it seemed a reasonable trade off.

If you work within the Google cloud ecosystem then all of their SDKs and tools use what they call default credentials which means they attempt to find some credentials on whatever machine you are running on. For example on a developer machine this will likely be some credential file in ~/.config/gcloud/ but on a k8s container running in GKE it will be credentials it acquires from making a request to an in-cluster metadata server. I think if you wanted to provide credentials explicitly then I think setting the env var GOOGLE_APPLICATION_CREDENTIALS to a file path to some credentials file also "just works" with all of their sdks.

@mrT23
Copy link
Collaborator

mrT23 commented Nov 15, 2023

@rhyst is this PR still active ?

@rhyst
Copy link
Author

rhyst commented Nov 15, 2023

Yes, I intend to update it 🙂

@rhyst
Copy link
Author

rhyst commented Nov 15, 2023

I have updated the documentation and config template. I will find some time to ensure all the commands work with the updated litellm (I have access to gpt3.5/gpt4 and vertex so I will test those).

@mrT23
Copy link
Collaborator

mrT23 commented Nov 15, 2023

Persistent review updated to latest commit 557ec72

@mrT23 mrT23 merged commit 388cc74 into Codium-ai:main Nov 15, 2023
@mrT23 mrT23 mentioned this pull request Nov 19, 2023
yochail pushed a commit to yochail/pr-agent that referenced this pull request Feb 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants