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

Generalize OpenAI Model Parser to allow other providers like Azure OpenAI #945

Open
Ankush-lastmile opened this issue Jan 16, 2024 · 3 comments

Comments

@Ankush-lastmile
Copy link
Member

Ankush-lastmile commented Jan 16, 2024

Currently the OpenAI Model parser only works with the default OpenAI as provider. Now that there are providers like Azure OpenAI ( authentication through oauth), we need to generalize the OpenAI Model parser to allow these different type of API providers.

One approach to this is to construct the OpenAI Client in the constructor of the OpenAI model parser, that way a user can simply extend the class and construct the client in the new constructor.

For Example:
openai.py

class OpenAIInference(ParameterizedModelParser):
    def __init__(self):
        super().__init__()
        # New code:
        self.client = OpenAI()

      async def run_inference():
           ...
          # Change this line to be self.client.completions.create(**completion_data)
252    response = openai.chat.completions.create(**completion_data) 

would allow for
SomeAzureOpenAIModelParser.py

class AzureOpenAIInference(OpenAIInference):
    def __init__(self):
        super().__init__()
        # New code:
        self.client = AzureOpenAI() # or whatever this should be

Also update unit tests as necessary.

Note: this applies to both python and typescript sdks.

Note: the provided example solution may not be the best way to solve this; would require credentials at construct time. All of the current model parsers require credentials at the run step

@saqadri
Copy link
Contributor

saqadri commented Jan 16, 2024

This won't work because of the way we register model parsers in config.py at initialization time.

I think it would be better to have an init function that sets up the model parser in the right way.
Also, AnyscaleEndpoint model parser uses the openai client, and the main OpenAI model parser should also use the OpenAI client. This will allow it to be configured in an init function.

The init function can be overridden by a derived class for things like azure openai usage.

@shuther
Copy link

shuther commented Mar 10, 2024

  1. What about leveraging litellm to fix this issue?
  2. Is it possible in the interim to set the base_url of the openai endpoint? We could still use litellm as an external component if needed. See example from github issue.

@Ankush-lastmile
Copy link
Member Author

Ankush-lastmile commented Mar 11, 2024

Hi @shuther, thanks for your suggestion. This issue was addressed by #1034 and can be marked as closed now. Providers such as AzureOpenAI, as mentioned in the original issue description, are now more seamlessly integrated the AIConfig SDK. Users can either initialize the OpenAI client with the base_url as you've suggested or set the relevant environment variables.

You can see an example of how this works inside this cookbook: https://github.com/lastmile-ai/aiconfig/tree/main/cookbooks/Claude-And-Azure

As for LiteLLM, it's on our radar as a potential out-of-the-box solution. Before I close this issue, I'd like to ensure we've fully addressed your needs. Are there specific concerns or goals you had in mind regarding the integration of LiteLLM?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants