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

[Azure] Support Realtime API #7

Closed
wants to merge 1 commit into from
Closed

Conversation

deyaaeldeen
Copy link
Owner

Support Azure Realtime API

In order to do so, this PR adds new AzureOpenAIRealtimeWebSocket and AzureOpenAIRealtimeWS clients

Key Changes

1. Introduce New open Method

  • Previously, the WebSocket connection was established in the class constructor, which could not handle async operations like fetching Azure AD tokens.
  • The new open method enables asynchronous fetching of the Azure AD token and establishes the WebSocket connection afterward.

2. New getAzureADToken Method in AzureOpenAI

  • Adds a getAzureADToken method in the AzureOpenAI class to provide refreshed Azure AD tokens.
  • The new method is called in the new clients to retrieve necessary Azure AD token for establishing the WebSocket connection

deployment: deploymentName,
});
const rt = new AzureOpenAIRealtimeWebSocket(client);
await rt.open();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're doing our own Azure class, do we still need the open() method? Wondering if we can, if we should decrease the delta between OAI and AOAI as much as possible

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we need to to wait for fetching a token before we open the socket and this is not possible inside a class constructor.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha, makes sense

}
return url;
}
const deploymentName = this.client.deploymentName ?? this.options.deploymentName;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quick check - if deployment name is set at the client, won't that add /deployments/deployment_name to the path? I wasn't able to support the scenario of a deployment name being passed at the client-level in Python so curious if there is something different here that lets JS do it.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if deployment name is set at the client, won't that add /deployments/deployment_name to the path?

Yes, the request path, not the baseURL one.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh right, now I remember that deployment name gets added to the base URL in buildRequest for JS and not in the client init (like Python). Cool, disregard this comment

@deyaaeldeen deyaaeldeen force-pushed the feat/azure-rt-internal branch from 31d3db3 to c3cc69c Compare January 22, 2025 23:05
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.

2 participants