-
Notifications
You must be signed in to change notification settings - Fork 12
Add GenericProvider #14
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
Conversation
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 don't think OpenAI has been GA yet so we probably should remove it.
- For xai, it is not appropriate to use the name MetaProvider. Instead, I think we should define a base class for GenericProvider for generic API type.
- MetaProvider could be kept for backwards compatibility, but the implementation will likely be inheritance from GenericProvider.
|
Hi @YouNeedCryDear, thanks for the comment. I made the following updates:
Here is a screenshot of the test:@Property |
|
I have updated the code to raise and error when customer endpoint is used without a provider. |
| def _get_provider(self, provider_map: Mapping[str, Any]) -> Any: | ||
| if self.provider is not None: | ||
| provider = self.provider | ||
| elif self.model_id.startswith(CUSTOM_ENDPOINT_PREFIX): |
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 order might need to change. You have to check the self.model_id is None first, otherwise, this line will raise exception.
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.
Sure. I moved up the logic to check if model_id is None.
BTW, is there a reason to allow model_id to be None?
It appears to me that the proper way to do model_id validation is not to allow None as default value in OCIGenAIBase. However, this is probably out of the scope here.
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 allowing model_id to be None was just an old langchain standard of how other vendor packages were doing. We sure can revisit the detail later.
| def _get_provider(self, provider_map: Mapping[str, Any]) -> Any: | ||
| if self.provider is not None: | ||
| provider = self.provider | ||
| elif self.model_id.startswith(CUSTOM_ENDPOINT_PREFIX): |
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 allowing model_id to be None was just an old langchain standard of how other vendor packages were doing. We sure can revisit the detail later.

As part of ongoing enhancements, the OCI Generative AI Service is introducing additional models from xAI and OpenAI. These new models adhere to the same API specifications as the Meta models; however, the existing implementation does not automatically derive the associated
providerfor xAI or OpenAI models.The
MetaProviderprovider can be used for xAI and OpenAI models provided by the OCI Generative AI service.This PR fixes #13 by adding a simple change to automatically use
MetaProviderwhen user is using xAI or OpenAI models.