Fix: base_model name for body and deplyment name in URL - #20747
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile OverviewGreptile SummaryThis PR updates Azure image generation so the request body can use The changes fit into the existing Azure image generation flow in Confidence Score: 2/5
|
| Filename | Overview |
|---|---|
| litellm/llms/azure/azure.py | Adjusts Azure image generation to use deployment name for URL and base_model for request body, but introduces a null-deref crash when litellm_params is None (litellm_params.get(...) in image_generation). |
| tests/test_litellm/llms/azure/image_generation/test_azure_image_generation_init.py | Adds sync/async tests for base_model vs deployment URL behavior, but one async test is defined as non-async and both tests swallow exceptions, risking false positives. |
Sequence Diagram
sequenceDiagram
participant Caller
participant AzureChatCompletion as AzureChatCompletion
participant AzureBaseURL as create_azure_base_url
participant SyncReq as make_sync_azure_httpx_request
participant AsyncReq as make_async_azure_httpx_request
alt Sync image_generation()
Caller->>AzureChatCompletion: image_generation(prompt, model=deployment, litellm_params.base_model)
AzureChatCompletion->>AzureChatCompletion: extra_body flatten + data.model = base_model || deployment
AzureChatCompletion->>AzureBaseURL: create_azure_base_url(model=deployment)
AzureBaseURL-->>AzureChatCompletion: img_gen_api_base (/openai/deployments/{deployment}/images/generations)
AzureChatCompletion->>SyncReq: POST(api_base=img_gen_api_base, json=data)
SyncReq-->>AzureChatCompletion: httpx.Response
AzureChatCompletion-->>Caller: ImageResponse
end
alt Async aimage_generation()
Caller->>AzureChatCompletion: aimage_generation(data.model=base_model, model=deployment)
AzureChatCompletion->>AzureBaseURL: create_azure_base_url(model=deployment)
AzureBaseURL-->>AzureChatCompletion: img_gen_api_base
AzureChatCompletion->>AsyncReq: POST(api_base=img_gen_api_base, json=data)
AsyncReq-->>AzureChatCompletion: httpx.Response
AzureChatCompletion-->>Caller: ImageResponse
end
| @pytest.mark.asyncio | ||
| async def test_azure_aimage_generation_base_model_vs_deployment_name(): | ||
| """ | ||
| Test that Azure async image generation correctly uses base_model in request body |
There was a problem hiding this comment.
Async test never runs
The async test is marked with @pytest.mark.asyncio but the function is defined with def (not async def). Pytest will treat it as a normal test and won’t await anything, so this doesn’t actually validate the async path.
This should be async def test_azure_aimage_generation_base_model_vs_deployment_name():.
There was a problem hiding this comment.
It is async def test_azure_aimage_generation_base_model_vs_deployment_name
| response = azure_chat_completion.image_generation( | ||
| prompt=prompt, | ||
| timeout=60.0, | ||
| optional_params=optional_params, | ||
| logging_obj=logging_obj, | ||
| headers={}, | ||
| model=model, | ||
| api_key=api_key, | ||
| api_base=api_base, | ||
| api_version=api_version, | ||
| litellm_params=litellm_params, | ||
| ) | ||
| except Exception as e: | ||
| # If there's an error, we still want to check the mock calls | ||
| pass |
There was a problem hiding this comment.
Test can falsely pass
Both new tests wrap the call in try/except Exception: pass and then only assert mock_request.called. This can still pass even if image_generation() fails before constructing the URL/body you want to verify (e.g., raises due to unrelated bug), and can hide regressions.
Remove the blanket exception handler (or assert on the specific expected exception) so the test actually fails when the call path breaks.
Additional Comments (1)
This needs a |
ceng-p
left a comment
There was a problem hiding this comment.
Looks like a decent work around, thanks
…odel_fix Fix: base_model name for body and deplyment name in URL
Relevant issues
Fixes #20741
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unitCI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Type
🆕 New Feature
🐛 Bug Fix
🧹 Refactoring
📖 Documentation
🚄 Infrastructure
✅ Test
Changes