-
Notifications
You must be signed in to change notification settings - Fork 30
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
✨ Add support for google gemini model. #307
Conversation
8999fc6
to
9a13fd5
Compare
@Myrausman thank you! Quick look at the PR and looked great. I'll help run through a manual test, but I don't expect to have time to help until Wednesday. |
@Myrausman Would you be able to start the server in demo mode (DEMO_MODE=true), which should dump a bunch of cached responses into |
I started server in demo mode but I'm not getting any cache new response for that particular model (pro-gemini). |
@Myrausman did you run: To get cached responses you need to send requests to the API, so that it can cache the response. You can read this to learn more of how to run: |
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.
Code changes look good...
Waiting on:
- Update of cached responses
- A maintainer to run a manual test to confirm working with Gemini
I am willing to test with a Gemini API key, I won't have time to help with that today, will try for helping tomorrow.
@Myrausman this will help to give more context of updating cached responses |
followin this file Contributing.md, when I'm running
|
I think you just need to clone a few sample repos paths Look at this link and run the fetch_apps.py it mentions: https://github.com/konveyor/kai/blob/main/docs/contrib/Dev_Environment.md#steps |
After running |
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 requirements.txt update looks incorrect.
@Myrausman did you run pip-compile
, you should have more dependencies added to requirements.txt to account for the langchain-google-genai package and it's dependencies.
Let's update requirements.txt and then I think we'll be in shape to merge this.
I confirmed that when I ran pip-compile to update requirements.txt I had success running run_example.py
.
I also confirmed that cached data is NOT being captured. I am less sure of why that is happening.
My thought is we move forward to merge this PR after the requirements.txt update is made, we accept we do not have cached data for gemini and work that as a separate issue, tracked as #315
interesting, I wonder if the google library is using a different library that isn't intercepted by VCR for managing the requests |
@Myrausman please take look at the merge conflicts we will need to resolve before we can merge. We merged another PR yesterday that altered the requirements a little, so you will need to reexamine requirements.in, add your changes in, rerun pip-compile and test, then update this PR with the changes. |
Signed-off-by: Myrausman <maira.usman5703@gmail.com>
Signed-off-by: Myrausman <maira.usman5703@gmail.com>
Signed-off-by: Myrausman <maira.usman5703@gmail.com>
Signed-off-by: Myrausman <maira.usman5703@gmail.com>
b0fedef
to
b30d496
Compare
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.
LGTM
Ran and verified functional on Mac with arm64
@Myrausman Thank you for your contribution! |
Fixes #304