feat: add Azure OpenAI support for embedding and VLM#808
feat: add Azure OpenAI support for embedding and VLM#808qin-ctx merged 2 commits intovolcengine:mainfrom
Conversation
|
|
Add `azure` as a first-class provider for both embedding models and VLM,
using the official `openai.AzureOpenAI` / `openai.AsyncAzureOpenAI` clients.
Changes:
- embedder: OpenAIDenseEmbedder now accepts `provider` and `api_version`
params; when provider is "azure", initializes AzureOpenAI client with
azure_endpoint and api_version
- VLM: OpenAIVLM switches between OpenAI and AzureOpenAI clients based
on the provider field; VLMFactory maps "azure" to OpenAIVLM
- config: EmbeddingModelConfig and VLMConfig gain `api_version` field;
embedding_config adds ("azure", "dense") factory entry with validation
- registry: add "azure" to VALID_PROVIDERS
- docs: update README_CN.md with Azure provider table entry, config
template fields, usage examples, and full config sample
Made-with: Cursor
2632358 to
9b2a58d
Compare
qin-ctx
left a comment
There was a problem hiding this comment.
Review Summary
Azure OpenAI provider support — overall direction is sound (reusing the OpenAI backend for Azure is the right call), but there's a validation gap in the VLM path and some improvement opportunities.
Blocking: 1 issue (missing api_base validation in VLM Azure path)
Non-blocking: 3 design/suggestion items (see inline comments)
Additional notes:
- No tests added for the new Azure provider path. Consider adding mock-based unit tests for: (a) Azure config correctly creates
AzureOpenAIclient with expected params; (b) missingapi_key/api_baseraises config validation errors; (c) factory routes"azure"toOpenAIVLM. - PR template checklist is unfilled — please mark the applicable items (Type of Change, Testing, etc.).
| client_kwargs: Dict[str, Any] = { | ||
| "api_key": self.api_key, | ||
| "azure_endpoint": self.api_base, | ||
| "api_version": self.api_version or "2025-01-01-preview", |
There was a problem hiding this comment.
[Bug] (blocking) Missing api_base validation for Azure provider.
The embedder correctly validates this (openai_embedders.py):
if not self.api_base:
raise ValueError("api_base (Azure endpoint) is required for Azure provider")But get_client() and get_async_client() pass self.api_base directly to azure_endpoint without checking. If api_base is None, AzureOpenAI(azure_endpoint=None) raises a confusing error about missing AZURE_OPENAI_ENDPOINT environment variable, rather than a clear configuration error.
Suggested fix — add validation before the Azure branch in both methods:
if self.provider == "azure":
if not self.api_base:
raise ValueError("api_base (Azure endpoint) is required for Azure provider")
...| client_kwargs["default_headers"] = self.extra_headers | ||
| self._sync_client = openai.OpenAI(**client_kwargs) | ||
| if self.provider == "azure": | ||
| client_kwargs: Dict[str, Any] = { |
There was a problem hiding this comment.
[Design] (non-blocking) The Azure/OpenAI client construction branching logic is nearly identical across three locations:
OpenAIDenseEmbedder.__init__()(openai_embedders.py)OpenAIVLM.get_client()(here)OpenAIVLM.get_async_client()(below, line 56)
Each repeats the same pattern: check provider → build azure_endpoint/api_version kwargs → handle extra_headers → instantiate client. Consider extracting a helper (e.g., _build_openai_client_kwargs(provider, api_key, api_base, api_version, extra_headers)) to reduce duplication and maintenance surface.
| "api_key": self.api_key, | ||
| "azure_endpoint": self.api_base, | ||
| "api_version": self.api_version or "2025-01-01-preview", | ||
| } |
There was a problem hiding this comment.
[Design] (non-blocking) The default api_version value "2025-01-01-preview" is hardcoded in three separate locations:
openai_embedders.py:125(embedder)- Here, line 37 (sync VLM client)
get_async_client(), line 60 (async VLM client)
When Azure releases a new API version, all three must be updated in sync. Consider extracting to a shared constant:
# e.g., in a shared module
DEFAULT_AZURE_API_VERSION = "2025-01-01-preview"| self._sync_client = None | ||
| self._async_client = None | ||
| self.provider = "openai" | ||
| self.api_version = config.get("api_version") |
There was a problem hiding this comment.
[Suggestion] (non-blocking) This replaces the previous self.provider = "openai" which was hardcoding the provider value, overriding the base class's self.provider = config.get("provider", "openai") (in VLMBase.__init__). Removing this override is correct — it's actually a bug fix that makes token usage tracking (in _update_token_usage_from_response and the streaming methods) accurately reflect the configured provider (e.g., "azure" instead of always "openai").
This behavioral change is worth mentioning in the PR description since it affects existing tracking behavior.
…constant - Extract `_build_openai_client_kwargs()` helper in openai_vlm.py to eliminate duplicated Azure/OpenAI client construction across get_client() and get_async_client() - Add `api_base` validation for Azure provider in VLM client methods (was missing, unlike the embedder which already validated it) - Extract `DEFAULT_AZURE_API_VERSION` constant in registry.py and reference it from both embedder and VLM, so future Azure API version bumps only require a single change - Note: removing the hardcoded `self.provider = "openai"` override in OpenAIVLM.__init__ (done in the previous commit) is an intentional bug fix — it allows VLMBase.provider to correctly reflect the configured value (e.g. "azure"), which fixes token usage tracking Made-with: Cursor
Add
azureas a first-class provider for both embedding models and VLM, using the officialopenai.AzureOpenAI/openai.AsyncAzureOpenAIclients.Changes:
providerandapi_versionparams; when provider is "azure", initializes AzureOpenAI client with azure_endpoint and api_versionapi_versionfield; embedding_config adds ("azure", "dense") factory entry with validationMade-with: Cursor
Description
Related Issue
Type of Change
Changes Made
Testing
Checklist
Screenshots (if applicable)
Additional Notes