Skip to content

Temporary fix for Azure OpenAI endpoints #23

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

Closed
wants to merge 1 commit into from
Closed

Temporary fix for Azure OpenAI endpoints #23

wants to merge 1 commit into from

Conversation

ugwun
Copy link
Contributor

@ugwun ugwun commented Jun 7, 2023

No description provided.

@ugwun ugwun mentioned this pull request Jun 7, 2023
Copy link
Owner

@CJCrafter CJCrafter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thankyou for making a Pull Request! I appreciate you handling the Azure stuff.

Needs some changes before I can merge this and update. I am definitely worried about future proofing the technology a bit since it is rapidly changing.

@@ -26,13 +26,15 @@ class AzureOpenAI @JvmOverloads constructor(
) : OpenAI(apiKey, organization, client) {

override fun buildRequest(request: Any, endpoint: String): Request {
val removedV1Endpoint = endpoint.removePrefix("v1/") // temporary fix for Azure, as it doesn't support v1/ in the url
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of removing the prefix from the endpoint, a slightly more version stable method would be to have overridable functions for endpoints in the OpenAI class. This way, if OpenAI moves something to V2 we can easily change that in code with creating clutter anywhere else.

AzureOpenAI can override these methods with their own endpoints, and can even throw an exception for unsupported endpoints.

val openai = AzureOpenAI(key)
val modelName = dotenv()["OPENAI_MODEL"]
val azureBaseUrl = dotenv()["OPENAI_AZURE_BASE_URL"]
val openai = AzureOpenAI(key, modelName = modelName, azureBaseUrl = azureBaseUrl)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Errors like this should definitely have a check in the constructor, or the arguments should be redone so the azure arguments come before the optional ones. A builder class is also an option.

@ugwun ugwun closed this Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants