-
Notifications
You must be signed in to change notification settings - Fork 14
Add support for OCI OpenAI Responses API with Langchain/Langgraph #61
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
base: main
Are you sure you want to change the base?
Add support for OCI OpenAI Responses API with Langchain/Langgraph #61
Conversation
|
Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application. When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated. If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public. |
| OUTPUT_VERSION = "responses/v1" | ||
|
|
||
|
|
||
| class OCIHttpxAuth(httpx.Auth): |
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.
I think the oci_openai has been published in github. Can we use that?
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.
sorry, u mean the internal SDK (OCI OpenAI Client SDK)? For LA this is not being published to pypi but only to internal artifactory. @anbhaduri
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.
We can only start taking dependency on oci_openai post GA when we start publishing it to pypi,
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.
Why cant we leverage this? https://github.com/oracle-samples/oci-openai/tree/v0.2.1
I don't think it is a good idea to have separate Auth defined.
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.
I just tested the oci-openai is already available thru pip install
https://docs.oracle.com/en-us/iaas/Content/generative-ai/oci-openai.htm
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.
we can take dependency on it. That repo needs to be modified to include things specific to oci openai response api for eg: conversation store id etc. Also the names of the headers are different for compartment id etc.
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.
If the users are expected to use oci-openai repo to interact with our service, then I think we should use it.
| super().__init__(signer=signer) | ||
|
|
||
|
|
||
| def get_base_url(region: str, override_url: str = "") -> str: |
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.
Those going to utils?
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.
you mean the existing utils under llms? we have kept it at a single place since its being only used by oci_generative_ai_responses_api module. wdyt?
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.
I think it is better to put into utils for extensibility, even for now it is only used for responses api.
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.
DONE
| ) | ||
|
|
||
|
|
||
| class OCIChatOpenAI(ChatOpenAI): |
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.
Why not following the naming standard? If the genai client is called ChatOCIGenAI, then I think this should better be ChatOCIOpenAI
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.
Renamed to ChatOCIOpenAI
| import requests | ||
| from langchain_openai import ChatOpenAI | ||
| from oci.config import DEFAULT_LOCATION, DEFAULT_PROFILE | ||
| from openai import ( |
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.
openai not required in dependencies?
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.
Based on the way current packages are managed, we give error messages when specific required packages are not installed. For eg: oracle-ads, langchain-openai. So we have followed the same. When langchain-openai is not installed an error message is thrown. langchain-openai brings in the openai as a transitive dependency. wdyt?
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.
It is going to fail because you are importing at the top level. Could you check what ads is doing.
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.
DONE
|
@YouNeedCryDear |
Sounds like a good idea. But this might be a huge change to the code base and I don't think the Reponses API team is able to do. |
Summary
Adds support for OpenAI Responses API by introducing a new class
OCIChatOpenAI. This allows users to invoke OCI Generative AI to leverage OpenAI Responses API.Changes
oci_generative_ai_responses_api.pywhich implementsOCIChatOpenAIclass and OCI Auth.examplesfolder to provide sample usagesREADME.mdTesting
Breaking Changes
None - this is a new feature that is fully backward compatible.