openai.ex icon indicating copy to clipboard operation
openai.ex copied to clipboard

add api_url param in init function

Open tiankonglan opened this issue 1 year ago • 5 comments

in some sence api_url is not "https://api.openai.com", so I commit the pr.

tiankonglan avatar May 30 '23 10:05 tiankonglan

Will this support Azure OpenAI API?

howard0su avatar May 31 '23 13:05 howard0su

Will this support Azure OpenAI API?

It doesn't look like it would support Azure because of the base URL configs for the various endpoints include v1. I think the v1 specific in the various endpoint modules would need to be moved to API base URI instead. https://github.com/mgallo/openai.ex/blob/ff48a96f60c3840aa08906706c670e0c21ad6de3/lib/openai/completions.ex#L6

talk2MeGooseman avatar Jun 09 '23 16:06 talk2MeGooseman

It would be amazing if this would support Azure. I'd be happy to help but it seems like a trivial change. Just dropping a vote in favour of moving that v1 to the API base URL. 👍

cigrainger avatar Jun 15 '23 12:06 cigrainger

Hi @tiankonglan! Thanks for this PR The api_url param in the config was only designed to be used as a test url (this is the original issue: https://github.com/mgallo/openai.ex/issues/20), However, is correct to have it in the GenServer init function, but in this PR implementation it will not consider the default value, I'll change it to:

  def init(_opts) do
    config = %{
      api_key: get_config_value(:api_key),
      organization_key: get_config_value(:organization_key),
      http_options: get_config_value(:organization_key),
      api_url: get_config_value(:api_url, @openai_url)
    }

    {:ok, config}
  end

and remove the @config_keys module attribute, WDYT?

Azure integration topic

@howard0su @talk2MeGooseman @cigrainger overriding api_url is not sufficient to fully support Azure APIs, because the azure auth token header differs from the standard one, as you can see here: https://learn.microsoft.com/en-us/azure/cognitive-services/openai/reference#example-request (api_key is passed as an api-key header). In the standard client (current implementation) the token is Authorization: Bearer OPENAI_API_KEY https://platform.openai.com/docs/api-reference/authentication.

To support azure in a proper way we should create a specific client for it, that share the logic of the existing one for handling the response, but pass specific url and headers for authentication.

I don't think that moving the v1 to the API base url is a good idea, because it can impact future releases of the APIs that can have different versions for different actions.

FYI I converted the original issue about Azure in a discussion thread here

mgallo avatar Jun 18 '23 15:06 mgallo

Hey @mgallo @howard0su @cigrainger @talk2MeGooseman, I started to work on the Azure support and I have something roughly working on a branch inside my fork, I currently have tested only the OpenAI.chat_completion because that was the endpoint I needed to put together a prototype. I didn't spend to much time on polishing the whole API wrapper.

If you are interested I can open a PR here where we can discuss what should be changed and added to have something mergeable.

mtanzi avatar Jun 27 '23 21:06 mtanzi