-
Notifications
You must be signed in to change notification settings - Fork 19
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
request_openai
removes the query parameter api-version
for an AzureProvider
#63
Comments
Thanks for filing the bug! Doesn't this belong to HTTP.jl if it's the HTTP behavior that's causing the error? EDIT: Note that |
It might not be a bug in |
@svilupp I have just checked the functionality of Setup:Create the provider: julia> const AZURE_OPENAI_API_KEY = "your-api-key"
julia> const AZURE_OPENAI_HOST = "https://<resource-name>.openai.azure.com"
julia> model = "gpt-4o-2"
julia> provider = OpenAI.AzureProvider(;
api_key = AZURE_OPENAI_API_KEY,
base_url = AZURE_OPENAI_HOST * "/openai/deployments/$model"
)
julia> provider.api_version
"2023-03-15-preview"
julia> OpenAI.build_url(provider, "chat/completions")
"https://<resource-name>.openai.azure.com/openai/deployments/gpt-4o-2/chat/completions?api-version=2023-03-15-preview"
Build the request: julia> messages = [Dict("role"=> "user", "content" => "How are you doing?")]
julia> body = JSON.json(Dict("messages" => messages))
julia> headers = Dict("Content-Type" => "application/json", "api-key" => AZURE_OPENAI_API_KEY)
|
I suspect it might be this override: Line 80 in 762daad
But it was introduced after I've used Azure API, so that might be what broke it. @cpfiffer, is there a reason to force the empty vector in |
Not that I can recall. I think to fix it we should probably have a query parser in there (or use whatever HTTP.jl does), but it's probably fine to just remove it and see what happens. Everything in there should be tested as far as I remember. |
My understanding is different -- that line should be removed. This is all handled natively by HTTP.jl. See the following: using HTTP
using HTTP.URIs
# used by `request_uri`
target = URI("https://httpbin.org/get?a=1&b=2")
@info "URIs" target.host target.path target.query
┌ Info: URIs
│ target.host = "httpbin.org"
│ target.path = "/get"
└ target.query = "a=1&b=2" If you leave query as nothing, URL package will extract it easily in the HTTP stack. I'd suggest opening a PR to remove this line. Anyone against it? |
Cc: @svilupp
While developing the support for Azure OpenAI in PromptingTools.jl, I came across the following issue. If I use the
OpenAI.create_chat
with anAzureProvider
the?api-version=2023-03-15-preview
is removed from the URL:See how even though the result of
OpenAI.build_url(provider, "chat/completions")
includes the query parameter:the URL used in the request doesn't:
To get the correct URL, I need to use
OpenAI.openai_request
pasing the argumentquery = Dict("api-version" => provider.api_version)
:The issue seems to come from the
HTTP.request
function, which removes the query parameters from the URL and only includes them if they are provided through thequery
argument. The issue doesn't occur when usingHTTP.post
.The text was updated successfully, but these errors were encountered: