Skip to content
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

fix(azure): remove unnecessary model parameter and require azure deployment #2123

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

Programmer-RD-AI
Copy link

@Programmer-RD-AI Programmer-RD-AI commented Feb 15, 2025

This PR addresses the reported issue with the Azure Realtime API URL generation (Issue #2120) by ensuring that the correct deployment value is used and by removing the unnecessary model query parameter. Previously, both the synchronous and asynchronous _configure_realtime methods included a model parameter that is not required by Azure, which could lead to confusion. With this update, only the deployment parameter is used, resulting in a cleaner and more accurate URL construction:

wss://<azure_endpoint>/openai/realtime?api-version=2024-10-01-preview&deployment=<azure_deployment>

Key Changes:

  • Deployment Enforcement:
    The code now raises a ValueError if an azure_deployment is not provided, preventing misconfigurations.
  • Query Parameter Simplification:
    The model parameter has been removed from the query dictionary in both _configure_realtime methods. Only the deployment parameter, taken from self._azure_deployment, is now used.
  • Code Consistency:
    Both synchronous and asynchronous methods now generate the URL consistently, matching the expected format for Azure realtime connections.
  • Verification:
    Tests confirm that the URL is correctly generated when an azure_deployment is provided and that an error is raised if it is missing.

All tests have passed, so the fix is verified to work as intended. Please review the changes and let me know if you have any further feedback!

@eavanvalkenburg
Copy link
Contributor

One question, afaik the query param for model isn't needed for azure, although the extra param doesn't cause a break, but not having to provide it to the connect method would be good!

@Programmer-RD-AI
Copy link
Author

One question, afaik the query param for model isn't needed for azure, although the extra param doesn't cause a break, but not having to provide it to the connect method would be good!

Hi @eavanvalkenburg, thanks for the suggestion. I've updated the PR to remove the unnecessary model query parameter from the _configure_realtime methods (both synchronous and asynchronous). Now, only the required deployment parameter is included when constructing the Azure realtime API URL.

@Programmer-RD-AI Programmer-RD-AI changed the title fix(azure): require azure deployment and update query parameters fix(azure): remove unnecessary model parameter and require azure deployment Feb 15, 2025
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.

3 participants