-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
base: main
Are you sure you want to change the base?
Conversation
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! |
…enforce azure deployment requirement
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. |
…ncluded in query parameters
…er for deployment assignment
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:
Key Changes:
The code now raises a
ValueError
if an azure_deployment is not provided, preventing misconfigurations.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.Both synchronous and asynchronous methods now generate the URL consistently, matching the expected format for Azure realtime connections.
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!