fix: make azure api-version query param optional#9221
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d1c246aa0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if let Some(version) = api_version { | ||
| api_client = api_client.with_query(vec![("api-version".to_string(), version)]); | ||
| } |
There was a problem hiding this comment.
Preserve api-version for legacy Azure deployment paths
from_env now omits the api-version query parameter when AZURE_OPENAI_API_VERSION is unset, but this provider still targets the legacy .../openai/deployments/{deployment}/... route shape (not /openai/v1/...). For classic Azure OpenAI endpoints, requests on that route require an API version, so a newly configured Azure provider (where this key is now optional) can fail at runtime with 4xx errors even though setup succeeds. Please keep a fallback for non-v1 endpoints (or infer based on endpoint path) so legacy deployments remain functional without manual env overrides.
Useful? React with 👍 / 👎.
Newer Azure OpenAI deployments using /v1 paths reject the api-version query parameter with a 400 error. Now we infer from the endpoint URL: - /v1 endpoints: omit api-version (unless explicitly set) - Legacy endpoints: default to 2024-10-21 (preserving existing behavior) Users can always override via AZURE_OPENAI_API_VERSION. Also removes the default hint from the ConfigKey so the desktop UI doesn't pre-fill a value for new-style endpoint users. Based on the approach from #8256 by @octo-patch. Fixes #8236 Signed-off-by: Douwe Osinga <douwe@squareup.com>
8d1c246 to
4f3cf4a
Compare
Newer Azure OpenAI deployments using
/v1paths reject theapi-versionquery parameter with a 400 error:This PR makes the
api-versionquery parameter conditional — it is only included whenAZURE_OPENAI_API_VERSIONis explicitly set by the user.Changes:
AZURE_DEFAULT_API_VERSIONconstant and the hardcoded fallbackapi-versionquery param whenAZURE_OPENAI_API_VERSIONis configuredConfigKeyto not be required and to have no default hint, so the desktop UI doesn't pre-fill a value that would re-introduce the problemBackward compatibility:
AZURE_OPENAI_API_VERSION=2024-10-21→ works as before/v1deployments: leave it unset → no query param, no 400 errorBased on the approach from #8256 by @octo-patch, with the additional fix of removing the default hint from the provider metadata so the UI doesn't suggest a default.
Fixes #8236
May also fix #8876